* [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end
@ 2022-02-08 20:32 Scott Theisen
2022-05-15 18:50 ` Scott Theisen
0 siblings, 1 reply; 6+ messages in thread
From: Scott Theisen @ 2022-02-08 20:32 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end
2022-02-08 20:32 [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end Scott Theisen
@ 2022-05-15 18:50 ` Scott Theisen
2022-09-16 17:41 ` Scott Theisen
0 siblings, 1 reply; 6+ messages in thread
From: Scott Theisen @ 2022-05-15 18:50 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end
2022-05-15 18:50 ` Scott Theisen
@ 2022-09-16 17:41 ` Scott Theisen
2022-09-22 14:59 ` Anton Khirnov
0 siblings, 1 reply; 6+ messages in thread
From: Scott Theisen @ 2022-09-16 17:41 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end
2022-09-16 17:41 ` Scott Theisen
@ 2022-09-22 14:59 ` Anton Khirnov
2022-09-22 18:26 ` Scott Theisen
0 siblings, 1 reply; 6+ messages in thread
From: Anton Khirnov @ 2022-09-22 14:59 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end
2022-09-22 14:59 ` Anton Khirnov
@ 2022-09-22 18:26 ` Scott Theisen
0 siblings, 0 replies; 6+ messages in thread
From: Scott Theisen @ 2022-09-22 18:26 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end
2022-02-04 15:16 [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_mpeg2: Use smaller scope for variables Andreas Rheinhardt
@ 2022-02-05 6:36 ` Scott Theisen
0 siblings, 0 replies; 6+ messages in thread
From: Scott Theisen @ 2022-02-05 6:36 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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".
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-22 18:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 20:32 [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end Scott Theisen
2022-05-15 18:50 ` Scott Theisen
2022-09-16 17:41 ` Scott Theisen
2022-09-22 14:59 ` Anton Khirnov
2022-09-22 18:26 ` Scott Theisen
-- strict thread matches above, loose matches on Subject: below --
2022-02-04 15:16 [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_mpeg2: Use smaller scope for variables Andreas Rheinhardt
2022-02-05 6:36 ` [FFmpeg-devel] [PATCH] cbs_mpeg2_split_fragment(): cache the buffer end Scott Theisen
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git