On 06/05/2025 00:37, Andreas Rheinhardt wrote:
> Patches attached.
>
> - Andreas
>
>
> [PATCH 05/11] avcodec/cbs,cbs_jpeg: Split ff_cbs_append_unit_data()
Suggest assert0 for all error cases in cbs generic code, since none of them are
inner-loop performance sensitive.
Patch looks good.
> [PATCH 06/11] avcodec/cbs: Allow to avoid refcounting/data copying
Having force_refcounting set to one at init makes me think the sense of it
should be other way around? ("disable_refcounting"?)
Would it be sensible for the new checks for data_ref to assert that you are not
in a refcounting case?
Also wondering whether we should have a new helper function something like:
int ff_cbs_make_unit_data_ref(SomethingContexty *c, AVBufferRef **ref, const
CodedBitstreamUnit *unit)
{
av_assert0(!*ref);
if (unit->data_ref) {
*ref = av_buffer_ref(unit->data_ref);
if (!*ref)
return AVERROR(ENOMEM);
} else {
av_assert0(c->disable_refcounting);
}
return 0;
}
to avoid the repeated code you add in the various codec files.
> [PATCH 07/11] avcodec/apv_parser: Don't use dummy AVBufferRef*
> [PATCH 08/11] Revert "avcodec/cbs: add an AVBufferRef input argument to
> ff_cbs_read()"
Yes, this ends up being a nicer answer.
> [PATCH 09/11] avcodec/av1_parser: Avoid copying data
It's not obvious that this does the right thing with the persistent reference
to the sequence header object. Can you explain further how that is working?
> [PATCH 10/11] avcodec/cbs_bsf: Avoid creating unnecessary references
Does this not add a significant new constraint on all bsfs using the cbs_bsf
structure? It's unclear to me whether works in all cases such as those where
new units are added.
> [PATCH 11/11] avcodec/bsf/trace_headers: Avoid creating unnecessary references
Same thought as 9 in other codecs, but fine if 9 is.
Thanks,
- Mark
_______________________________________________
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".
