Message ID | tencent_14EA9C130B1C5C0BB526255088E6C967D806@qq.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/5] avcodec/h264_mp4toannexb_bsf: refactor start_code_size handling | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
Ping for the patchset. > On May 20, 2023, at 00:41, 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; > > 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".
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 > > 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".
> 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".
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; 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;
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(-)