[FFmpeg-devel] [PATCH] avcodec/h264_metadata: add change pic_order_cnt_type option

[Andreas Rheinhardt]

sharpbai at gmail.com:
> To Andreas Rheinhardt,
> 
>> This is wrong. Instead you should make the SPS that you are modifying
>> writable and update it. (The h264_metadata bsf uses separate
>> CodedBitstreamContexts for input and output and the SPS is normally
>> shared between them, so if someone wants to modify a field one needs to
>> make it writable which makes a copy of the NAL unit (the reason it is
>> currently not done is because the fields that are changed are not used
>> for parsing at all, but pic_order_cnt_type is). See h264_redundant_pps
>> for an example of this.)
>>
>> (Btw: What you are doing here will break writing H.264 SPS if
>> pic_order_cnt_type_write isn't set (unless pic_order_cnt_type is zero).
>> This can e.g. happen for the aforementioned h264_redundant_pps BSF.)
> 
> Got it. I will implement a new bsf using the The right way.
> 
> 
>> This may break valid files and is therefore completely unsafe and should
>> IMO not be in this bsf.
>> Have you tested whether setting the number of reorder frames (which is
>> the typical way to set the delay in H.264) would be enough for your
>> usecase? I have already made a patch for this here:
>> https://github.com/mkver/FFmpeg/commit/6f6de261b30ef493f65af293c66798fe625ea743,
>> but it is not based upon current git master. Just remove the check
>> (which relies on a function not added in git master (introduced here:
>> https://github.com/mkver/FFmpeg/commit/29ff08b47ca63faff250802cdf743e2ac89d34f5)).
>> My usecase for this patch is slightly different from yours though: When
>> the input doesn't have reorder frames set, one has to guess it if the
>> container does not provide dts (think of Matroska). And said guess can
>> be wrong.
> 
> I have read the patch above. It could not solve my case. In my case, the
> keypoint is we do know the encoder will not reorder the encoded frame,
> but the decoder could not recognize it. Then for safety some decoders will
> allocate a decode buffer larger than we need, which result in decoding delay.
> At realtime scenario, this delay is not accepted. The patch above will not
> accept a smaller value (such as 0) to minimize the delay, so it could not
> solve my case. 

What does "will not accept a smaller value (such as 0)" mean? Does it
error out? What would happen if you set
sps->vui.max_dec_frame_buffering = FFMAX(ctx->reorder_frames,
sps->max_num_ref_frames); instead of what it is set to know (if the
bitstream restriction flags aren't there, max_dec_frame_buffering is
inferred to be 16 (the maximum size) and will stay 16 with the old code).
Notice that the change I am suggesting here is not safe in general.

> 
> Yes, there is a risk of breaking valid files. I will raise an error when detecting
> a reordered frame on setting pic_order_cnt_type to 2 to avoid breaking valid
> files.
> 
>>
>> _______________________________________________
>> 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".
>