Re: [FFmpeg-devel] [PATCH 1/5] avcodec/h264_mp4toannexb_bsf: refactor start_code_size handling

["\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>]

> On May 29, 2023, at 23:54, mypopy@gmail.com wrote:
> 
> On Fri, May 19, 2023 at 4:41 PM Zhao Zhili <quinkblack@foxmail.com> wrote:
>> 
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> start_code_size depends on whether PS comes from out-of-band or
>> in-band. Make the code more readable.
>> ---
>> libavcodec/h264_mp4toannexb_bsf.c | 34 ++++++++++++++++++++++++-------
>> 1 file changed, 27 insertions(+), 7 deletions(-)
>> 
>> diff --git a/libavcodec/h264_mp4toannexb_bsf.c b/libavcodec/h264_mp4toannexb_bsf.c
>> index d11be455c2..7dce1ae9b6 100644
>> --- a/libavcodec/h264_mp4toannexb_bsf.c
>> +++ b/libavcodec/h264_mp4toannexb_bsf.c
>> @@ -43,10 +43,26 @@ typedef struct H264BSFContext {
>>     int      extradata_parsed;
>> } H264BSFContext;
>> 
>> +enum PsSource {
>> +    PS_OUT_OF_BAND = -1,
>> +    PS_NONE = 0,
>> +    PS_IN_BAND = 1,
>> +};
>> +
>> static void count_or_copy(uint8_t **out, uint64_t *out_size,
>> -                          const uint8_t *in, int in_size, int ps, int copy)
>> +                          const uint8_t *in, int in_size, enum PsSource ps, int copy)
>> {
>> -    uint8_t start_code_size = ps < 0 ? 0 : *out_size == 0 || ps ? 4 : 3;
>> +    uint8_t start_code_size;
>> +
>> +    if (ps == PS_OUT_OF_BAND)
>> +        /* start code already present in out-of-band ps data, so don't need to
>> +         * add it manually again
>> +         */
>> +        start_code_size = 0;
>> +    else if (ps == PS_IN_BAND || *out_size == 0)
>> +        start_code_size = 4;
>> +    else
>> +        start_code_size = 3;
> I prefer the original style, they are shorter and more concise

It takes a little time to figure out what two ternary do in a
single statement, it takes more time to figure out why, and
multiple ternaries is hard to write comments properly.

Multiple ternaries in a single statement works in simple cases.
For current situation, I prefer ‘if-else’.

>> 
>>     if (copy) {
>>         memcpy(*out + start_code_size, in, in_size);
>> @@ -202,6 +218,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
>> 
>>         do {
>>             uint32_t nal_size = 0;
>> +            enum PsSource ps;
>> 
>>             /* possible overread ok due to padding */
>>             for (int i = 0; i < s->length_size; i++)
>> @@ -230,7 +247,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
>>                     if (!s->sps_size) {
>>                         LOG_ONCE(ctx, AV_LOG_WARNING, "SPS not present in the stream, nor in AVCC, stream may be unreadable\n");
>>                     } else {
>> -                        count_or_copy(&out, &out_size, s->sps, s->sps_size, -1, j);
>> +                        count_or_copy(&out, &out_size, s->sps, s->sps_size, PS_OUT_OF_BAND, j);
>>                         sps_seen = 1;
>>                     }
>>                 }
>> @@ -246,19 +263,22 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt)
>>             if (new_idr && unit_type == H264_NAL_IDR_SLICE && !sps_seen && !pps_seen) {
>>                 if (ctx->par_out->extradata)
>>                     count_or_copy(&out, &out_size, ctx->par_out->extradata,
>> -                                  ctx->par_out->extradata_size, -1, j);
>> +                                  ctx->par_out->extradata_size, PS_OUT_OF_BAND, j);
>>                 new_idr = 0;
>>             /* if only SPS has been seen, also insert PPS */
>>             } else if (new_idr && unit_type == H264_NAL_IDR_SLICE && sps_seen && !pps_seen) {
>>                 if (!s->pps_size) {
>>                     LOG_ONCE(ctx, AV_LOG_WARNING, "PPS not present in the stream, nor in AVCC, stream may be unreadable\n");
>>                 } else {
>> -                    count_or_copy(&out, &out_size, s->pps, s->pps_size, -1, j);
>> +                    count_or_copy(&out, &out_size, s->pps, s->pps_size, PS_OUT_OF_BAND, j);
>>                 }
>>             }
>> 
>> -            count_or_copy(&out, &out_size, buf, nal_size,
>> -                          unit_type == H264_NAL_SPS || unit_type == H264_NAL_PPS, j);
>> +            if (unit_type == H264_NAL_SPS || unit_type == H264_NAL_PPS)
>> +                ps = PS_IN_BAND;
>> +            else
>> +                ps = PS_NONE;
>> +            count_or_copy(&out, &out_size, buf, nal_size, ps, j);
>>             if (!new_idr && unit_type == H264_NAL_SLICE) {
>>                 new_idr  = 1;
>>                 sps_seen = 0;
>> --
>> 2.25.1
>> 
>> _______________________________________________
>> 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".
> 
> 
> 
> -- 
> =======================================
> Jun zhao/赵军
> +++++++++++++++++++++++++++++++++++++++
> _______________________________________________
> 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 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".