Yes, your patch solves my problem too, which is very nice. But I still think AV_PKT_DATA_NEW_EXTRADATA is not just data, it is also an event. As sps/pps data it can be removed, but as event it should be reserved.
Anyway, your patch should be merged. On Fri, Apr 25, 2025 at 1:24 AM Zhao Zhili < quinkblack-at-foxmail....@ffmpeg.org> wrote: > On Apr 24, 2025, at 22:53, jie jiang <jiangjie...@gmail.com> wrote: > > > > ctx->par_out->extradata isn't updated by this commit. Only > NEW_EXTRADATA > > is updated (avvc to annexb). > > > > You suggest the patch should only remove AV_PKT_DATA_NEW_EXTRADATA from > > packet, but AV_PKT_DATA_NEW_EXTRADATA is used to notify the codec or the > > format that the extradata buffer was changed and the receiving side > should > > act upon it appropriately. So i think NEW_EXTRADATA should be reserved. > > h264_mp4toannexb is supposed to convert out-of-band PS in avcc format to > in-band > annex format, for decoder and for muxer like mpegts. It can be confusing > to keep > another group of duplicated PS in side data. > > And we have extract_extradata bsf for any case NEW_EXTRADATA in side data > is needed. > > > > > If you think we don't need support mixed bitstream format. maybe this > > patch that i commit should be rejected. > > We don’t support mixed bitstream format. The issue is totally valid and > needs to be fixed. > Here is my patch: > > https://ffmpeg.org/pipermail/ffmpeg-devel/2025-April/342723.html > > > > > On Thu, Apr 24, 2025 at 10:21 PM Zhao Zhili < > > quinkblack-at-foxmail....@ffmpeg.org> wrote: > > > >> > >> > >>> On Apr 24, 2025, at 20:42, jiangjie <jiangjie...@gmail.com> wrote: > >>> > >>> if get AV_PKT_DATA_NEW_EXTRADATA in AVPacket, this bitstream filter > will > >> return an error. > >>> ffmpeg -i rtmp://xxx/live/xxx -bsf:v > >> "h264_mp4toannexb,h264_metadata=aud=remove" -c copy -f null - > >> > >> The subject and commit message is confusing. > >> > >> ’this’ isn’t a bitstream filter, it’s a bsf chain. And what report error > >> is h264_metadata, not h264_mp4toannexb. > >> > >> The subject can be: Remove NEW_EXTRADATA sidedata > >> > >> And commit message can be: > >> > >> We don’t support mixed bitstream format. For example, h264_metadata > report > >> error in this case > >> > >> > >>> --- > >>> libavcodec/bsf/h264_mp4toannexb.c | 26 ++++++++++++++----- > >>> tests/fate/h264.mak | 3 +++ > >>> .../fate/h264-bsf-mp4toannexb-h264_metadata | 12 +++++++++ > >>> 3 files changed, 35 insertions(+), 6 deletions(-) > >>> create mode 100644 tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata > >>> > >>> diff --git a/libavcodec/bsf/h264_mp4toannexb.c > >> b/libavcodec/bsf/h264_mp4toannexb.c > >>> index dda064287e..42a5a274bf 100644 > >>> --- a/libavcodec/bsf/h264_mp4toannexb.c > >>> +++ b/libavcodec/bsf/h264_mp4toannexb.c > >>> @@ -252,14 +252,21 @@ static int > >> h264_mp4toannexb_filter_ps(H264BSFContext *s, > >>> return 0; > >>> } > >>> > >>> +static int is_annexb(const uint8_t *extradata, int extra_size) { > >>> + if (!extra_size || (extra_size >= 3 && AV_RB24(extradata) == 1) || > >>> + (extra_size >= 4 && AV_RB32(extradata) == 1)) { > >>> + return 1; > >>> + } else { > >>> + return 0; > >>> + } > >>> +} > >>> + > >>> static int h264_mp4toannexb_init(AVBSFContext *ctx) > >>> { > >>> int extra_size = ctx->par_in->extradata_size; > >>> > >>> /* retrieve sps and pps NAL units from extradata */ > >>> - if (!extra_size || > >>> - (extra_size >= 3 && AV_RB24(ctx->par_in->extradata) == 1) || > >>> - (extra_size >= 4 && AV_RB32(ctx->par_in->extradata) == 1)) { > >>> + if (is_annexb(ctx->par_in->extradata, extra_size)) { > >>> av_log(ctx, AV_LOG_VERBOSE, > >>> "The input looks like it is Annex B already\n"); > >>> } else if (extra_size >= 7) { > >>> @@ -294,9 +301,16 @@ static int h264_mp4toannexb_filter(AVBSFContext > >> *ctx, AVPacket *opkt) > >>> extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA, > >>> &extradata_size); > >>> if (extradata) { > >>> - ret = h264_extradata_to_annexb(ctx, extradata, > extradata_size); > >>> - if (ret < 0) > >>> - goto fail; > >>> + if (!is_annexb(extradata, extradata_size) && extradata_size >= > >> 7) { > >>> + ret = h264_extradata_to_annexb(ctx, extradata, > >> extradata_size); > >>> + if (ret < 0) > >>> + goto fail; > >>> + av_packet_side_data_remove(in->side_data, > >> &in->side_data_elems, > >>> + AV_PKT_DATA_NEW_EXTRADATA); > >>> + uint8_t *data = av_packet_new_side_data( > >>> + in, AV_PKT_DATA_NEW_EXTRADATA, > >> ctx->par_out->extradata_size); > >>> + memcpy(data, ctx->par_out->extradata, > >> ctx->par_out->extradata_size); > >>> + } > >> > >> Remove adding AV_PKT_DATA_NEW_EXTRADATA. There’re in-band PS now. > >> > >> There is another case which not belonging to this patch. With > >> AV_PKT_DATA_NEW_EXTRADATA > >> in annexb format, it should be inserted as in-band PS and also be > removed > >> after. > >> > >>> } > >>> > >>> /* nothing to filter */ > >>> diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak > >>> index 00b153cc93..7b3e865955 100644 > >>> --- a/tests/fate/h264.mak > >>> +++ b/tests/fate/h264.mak > >>> @@ -231,6 +231,7 @@ FATE_H264-$(call DEMMUX, MOV, H264, > >> H264_MP4TOANNEXB_BSF) += fate-h264-bsf-mp4to > >>> > >> fate-h264-bsf-mp4toannexb-2 \ > >>> > >> fate-h264_mp4toannexb_ticket5927 \ > >>> > >> fate-h264_mp4toannexb_ticket5927_2 \ > >>> + > >> fate-h264-bsf-mp4toannexb-h264_metadata \ > >>> > >>> FATE_H264-$(call DEMMUX, H264, MOV, DTS2PTS_BSF) += > fate-h264-bsf-dts2pts > >>> > >>> @@ -441,6 +442,8 @@ fate-h264-bsf-mp4toannexb-2: > >> CMD = md5 -i $(TARGET_SAMPLES) > >>> fate-h264-bsf-mp4toannexb-2: CMP = oneline > >>> fate-h264-bsf-mp4toannexb-2: REF = > >> cffcfa6a2d0b58c9de1f5785f099f41d > >>> fate-h264-bsf-mp4toannexb-new-extradata: CMD = stream_remux > mov > >> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov "" h264 "-map > 0:v" > >>> +fate-h264-bsf-mp4toannexb-h264_metadata: CMD = transcode mov > >> $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov \ > >>> + h264 "-c:v > copy > >> -bsf:v h264_mp4toannexb,h264_metadata=aud=remove" "-c:v copy" > >>> fate-h264-bsf-dts2pts: CMD = transcode > "h264" > >> $(TARGET_SAMPLES)/h264-conformance/CAPAMA3_Sand_F.264 \ > >>> mov "-c:v copy > >> -bsf:v dts2pts -frames:v 50" "-c:v copy" > >>> fate-h264_mp4toannexb_ticket5927: CMD = transcode "mp4" > >> $(TARGET_SAMPLES)/h264/thezerotheorem-cut.mp4 \ > >>> diff --git a/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata > >> b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata > >>> new file mode 100644 > >>> index 0000000000..d3ba7f1886 > >>> --- /dev/null > >>> +++ b/tests/ref/fate/h264-bsf-mp4toannexb-h264_metadata > >>> @@ -0,0 +1,12 @@ > >>> +a48f175bf3458410d1dd1907682b92d3 > >> *tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264 > >>> +4642 tests/data/fate/h264-bsf-mp4toannexb-h264_metadata.h264 > >>> +#extradata 0: 38, 0x84d70914 > >>> +#tb 0: 1/1200000 > >>> +#media_type 0: video > >>> +#codec_id 0: h264 > >>> +#dimensions 0: 256x128 > >>> +#sar 0: 1/1 > >>> +0, -96000, -9223372036854775808, 48000, 2331, 0xc97509cb > >>> +0, -48000, -9223372036854775808, 48000, 64, 0x6ad91f4e, > >> F=0x0 > >>> +0, 0, -9223372036854775808, 48000, 2203, 0xe376d2a9 > >>> +0, 48000, -9223372036854775808, 48000, 44, 0x687a10ea, > >> F=0x0 > >>> -- > >>> 2.49.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-requ...@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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
