[FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end

[FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end

[Scott Theisen]

Also add a few clarifying comments.
---
 libavcodec/cbs_mpeg2.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 33bd3e0998..47732562d1 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -144,12 +144,12 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
                                     CodedBitstreamFragment *frag,
                                     int header)
 {
-    const uint8_t *start;
+    const uint8_t *start = frag->data;
+    const uint8_t * const buf_end = frag->data + frag->data_size;
     uint32_t start_code = -1;
     int err;
 
-    start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
-                                   &start_code);
+    start = avpriv_find_start_code(start, buf_end, &start_code);
     if (start_code >> 8 != 0x000001) {
         // No start code found.
         return AVERROR_INVALIDDATA;
@@ -165,12 +165,11 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         // start code in any way (as e.g. happens when there is a
         // Sequence End unit at the very end of a packet).
         start_code = UINT32_MAX;
-        end = avpriv_find_start_code(start--, frag->data + frag->data_size,
-                                     &start_code);
-
-        // start points to the byte containing the start_code_identifier
+        end = avpriv_find_start_code(start, buf_end, &start_code);
+        start--;
+        // decrement so start points to the byte containing the start_code_identifier
         // (may be the last byte of fragment->data); end points to the byte
-        // following the byte containing the start code identifier (or to
+        // following the byte containing the next start code identifier (or to
         // the end of fragment->data).
         if (start_code >> 8 == 0x000001) {
             // Unit runs from start to the beginning of the start code
@@ -178,6 +177,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
             unit_size = (end - 4) - start;
         } else {
            // We didn't find a start code, so this is the final unit.
+           // There is no start code to remove from end, hence not (end - 4).
            unit_size = end - start;
         }
 
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end

[Scott Theisen]

Ping; it still applies cleanly.

-Scott Theisen

On 2/8/22 15:32, Scott Theisen wrote:
> Also add a few clarifying comments.
> ---
>   libavcodec/cbs_mpeg2.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 33bd3e0998..47732562d1 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -144,12 +144,12 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>                                       CodedBitstreamFragment *frag,
>                                       int header)
>   {
> -    const uint8_t *start;
> +    const uint8_t *start = frag->data;
> +    const uint8_t * const buf_end = frag->data + frag->data_size;
>       uint32_t start_code = -1;
>       int err;
>   
> -    start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
> -                                   &start_code);
> +    start = avpriv_find_start_code(start, buf_end, &start_code);
>       if (start_code >> 8 != 0x000001) {
>           // No start code found.
>           return AVERROR_INVALIDDATA;
> @@ -165,12 +165,11 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>           // start code in any way (as e.g. happens when there is a
>           // Sequence End unit at the very end of a packet).
>           start_code = UINT32_MAX;
> -        end = avpriv_find_start_code(start--, frag->data + frag->data_size,
> -                                     &start_code);
> -
> -        // start points to the byte containing the start_code_identifier
> +        end = avpriv_find_start_code(start, buf_end, &start_code);
> +        start--;
> +        // decrement so start points to the byte containing the start_code_identifier
>           // (may be the last byte of fragment->data); end points to the byte
> -        // following the byte containing the start code identifier (or to
> +        // following the byte containing the next start code identifier (or to
>           // the end of fragment->data).
>           if (start_code >> 8 == 0x000001) {
>               // Unit runs from start to the beginning of the start code
> @@ -178,6 +177,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>               unit_size = (end - 4) - start;
>           } else {
>              // We didn't find a start code, so this is the final unit.
> +           // There is no start code to remove from end, hence not (end - 4).
>              unit_size = end - start;
>           }
>   

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end

[Scott Theisen]

Ping; it still applies cleanly.

-Scott Theisen

On 5/15/22 14:50, Scott Theisen wrote:
> Ping; it still applies cleanly.
>
> -Scott Theisen
>
> On 2/8/22 15:32, Scott Theisen wrote:
>> Also add a few clarifying comments.
>> ---
>>   libavcodec/cbs_mpeg2.c | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
>> index 33bd3e0998..47732562d1 100644
>> --- a/libavcodec/cbs_mpeg2.c
>> +++ b/libavcodec/cbs_mpeg2.c
>> @@ -144,12 +144,12 @@ static int 
>> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>>                                       CodedBitstreamFragment *frag,
>>                                       int header)
>>   {
>> -    const uint8_t *start;
>> +    const uint8_t *start = frag->data;
>> +    const uint8_t * const buf_end = frag->data + frag->data_size;
>>       uint32_t start_code = -1;
>>       int err;
>>   -    start = avpriv_find_start_code(frag->data, frag->data + 
>> frag->data_size,
>> -                                   &start_code);
>> +    start = avpriv_find_start_code(start, buf_end, &start_code);
>>       if (start_code >> 8 != 0x000001) {
>>           // No start code found.
>>           return AVERROR_INVALIDDATA;
>> @@ -165,12 +165,11 @@ static int 
>> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>>           // start code in any way (as e.g. happens when there is a
>>           // Sequence End unit at the very end of a packet).
>>           start_code = UINT32_MAX;
>> -        end = avpriv_find_start_code(start--, frag->data + 
>> frag->data_size,
>> -                                     &start_code);
>> -
>> -        // start points to the byte containing the 
>> start_code_identifier
>> +        end = avpriv_find_start_code(start, buf_end, &start_code);
>> +        start--;
>> +        // decrement so start points to the byte containing the 
>> start_code_identifier
>>           // (may be the last byte of fragment->data); end points to 
>> the byte
>> -        // following the byte containing the start code identifier 
>> (or to
>> +        // following the byte containing the next start code 
>> identifier (or to
>>           // the end of fragment->data).
>>           if (start_code >> 8 == 0x000001) {
>>               // Unit runs from start to the beginning of the start code
>> @@ -178,6 +177,7 @@ static int 
>> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>>               unit_size = (end - 4) - start;
>>           } else {
>>              // We didn't find a start code, so this is the final unit.
>> +           // There is no start code to remove from end, hence not 
>> (end - 4).
>>              unit_size = end - start;
>>           }
>

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end

[Anton Khirnov]

Quoting Scott Theisen (2022-09-16 19:41:43)
> Ping; it still applies cleanly.

Missing motivation for this change. Is it faster, or why is the new code
better?

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end

[Scott Theisen]

On 9/22/22 10:59, Anton Khirnov wrote:
> Quoting Scott Theisen (2022-09-16 19:41:43)
>> Ping; it still applies cleanly.
> Missing motivation for this change. Is it faster, or why is the new code
> better?
>

To make it easier to read.  `buf_end` is shorter than `frag->data + 
frag->data_size`.  Initialize `start` to `frag->data` so the calls to 
avpriv_find_start_code are the same.

I realize readability is an opinion, but I felt that saving the buffer 
end to a variable and tweaking the comments increased the readability of 
the code.

Regards,

Scott
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end

[Scott Theisen]

It is constant, so instead of recalculating it every time, only
calculate it once.

Also add a few clarifying comments.
---
 libavcodec/cbs_mpeg2.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index dfea12ef1b..d3204b885a 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -144,12 +144,12 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
                                     CodedBitstreamFragment *frag,
                                     int header)
 {
-    const uint8_t *start;
+    const uint8_t *start = frag->data;
+    const uint8_t * const buf_end = frag->data + frag->data_size;
     uint32_t start_code = -1;
     int err;
 
-    start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
-                                   &start_code);
+    start = avpriv_find_start_code(start, buf_end, &start_code);
     if (start_code >> 8 != 0x000001) {
         // No start code found.
         return AVERROR_INVALIDDATA;
@@ -165,10 +165,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         // start code in any way (as e.g. happens when there is a
         // Sequence End unit at the very end of a packet).
         start_code = UINT32_MAX;
-        end = avpriv_find_start_code(start--, frag->data + frag->data_size,
-                                     &start_code);
-
-        // start points to the byte containing the start_code_identifier
+        end = avpriv_find_start_code(start, buf_end, &start_code);
+        start--;
+        // decrement so start points to the byte containing the start_code_identifier
         // (may be the last byte of fragment->data); end points to the byte
         // following the byte containing the start code identifier (or to
         // the end of fragment->data).
@@ -178,6 +177,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
             unit_size = (end - 4) - start;
         } else {
            // We didn't find a start code, so this is the final unit.
+           // There is no start code to remove from end, hence not (end - 4).
            unit_size = end - start;
         }
 
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".