[FFmpeg-devel] [PATCH] avcodec/bsf/h264_mp4toannexb.c: change extradata to annexb if this is avcc.
Zhao Zhili
quinkblack at foxmail.com
Thu Apr 24 20:23:55 EEST 2025
On Apr 24, 2025, at 22:53, jie jiang <jiangjie618 at 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.com at ffmpeg.org> wrote:
>
>>
>>
>>> On Apr 24, 2025, at 20:42, jiangjie <jiangjie618 at 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 at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list