Re: [FFmpeg-devel] movenc: add write_btrt option

["\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>]

> On Apr 8, 2022, at 4:36 AM, Jan Ekström <jeebjp@gmail.com> wrote:
> 
> On Thu, Apr 7, 2022 at 11:42 AM Eran Kornblau <eran.kornblau@kaltura.com> wrote:
>> 
>>> 
>>> 
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of "zhilizhao(???)"
>>> Sent: Wednesday, 6 April 2022 11:46
>>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>>> Subject: Re: [FFmpeg-devel] movenc: add write_btrt option
>>> 
>>>> On Apr 3, 2022, at 1:07 PM, Eran Kornblau <eran.kornblau@kaltura.com> wrote:
>>>> 
>>>> Trying my luck in a new thread…
>>>> 
>>>> This patch is in continuation to this discussion –
>>>> https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmp
>>>> eg.org%2Fpipermail%2Fffmpeg-devel%2F2022-March%2F294623.html&amp;data=
>>>> 04%7C01%7C%7Cb9907958f97048f5645708da17a9f3c8%7C0c503748de3f4e2597e268
>>>> 19d53a42b6%7C1%7C0%7C637848315958196733%7CUnknown%7CTWFpbGZsb3d8eyJWIj
>>>> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am
>>>> p;sdata=flQc21b5EVWTy%2Bkmw%2FncIWzdLqUxY5XFislPRs5Ij6o%3D&amp;reserve
>>>> d=0
>>>> 
>>>> supports forcing or disabling the writing of the btrt atom.
>>>> the default behavior is to write the atom only for mp4 mode.
>>>> ---
>>>> libavformat/movenc.c | 30 +++++++++++++++++++-----------
>>>> libavformat/movenc.h |  1 +
>>>> 2 files changed, 20 insertions(+), 11 deletions(-)
>>>> 
>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c index
>>>> 4c868919ae..b75f1c6909 100644
>>>> --- a/libavformat/movenc.c
>>>> +++ b/libavformat/movenc.c
>>>> 
>>> […]
>>>> 
>>>> -    if (track->mode == MODE_MP4 &&
>>>> -            ((ret = mov_write_btrt_tag(pb, track)) < 0))
>>>> -        return ret;
>>>> +    if ((mov->write_btrt == -1 && track->mode == MODE_MP4) || mov->write_btrt == 1) {
>>>> +        if ((ret = mov_write_btrt_tag(pb, track)) < 0) {
>>>> +            return ret;
>>>> +        }
>>>> +    }
>>> 
>>> I prefer to handle the auto mode (mov->write_btrt == -1) in a single place, so we don’t need to change multiple lines if the condition changed, e.g., enable btrt for MODE_MOV. Please correct me if I’m wrong, mov_init() has all of the contexts to overwrite mov->write_btrt.
>>> 
>> Makes sense, thanks for the feedback!
>> Updated patch attached
>> 
>> Eran
> 
> Generally speaking I am not against this patch. Would have possibly
> came up with something similar myself in case the actual output of
> libavformat would have caused issues, which surprisingly enough it
> wasn't.
> 
> I know you have just copied the logic from tmcd or so, but I think the
> -1 logic is unnecessary. We don't need to force writing of this
> structure no matter what, so you either have it enabled (by default),
> or disabled. If additional formats such as QTFF have since added the
> btrt box into their specification, that doesn't need forcing, but
> rather addition of it into the logic later (if you wanted forcing then
> you'd have to deal with strict_std_compliance being
> unofficial/experimental or higher etc, and if this was not set -
> warning the user that a not officially defined functionality was being
> written into the container and exiting with AVERROR_EXPERIMENTAL).
> 
> Additionally, I thought new options go to the end of the AVOption
> array, but then saw 1dddb930aaf0cadaa19f86e81225c9c352745262 where
> James added "crf" into the middle of an array... so I guess since it's
> an array and not a struct the location no longer matters as much?
> ┐(´д｀)┌ Although the struct integer should definitely go to the end of
> it, otherwise you are breaking existing offsets? Although thankfully,
> the struct isn't externally exposed so someone else could chime in
> regarding this, I am unfortunately quite tired throughout this week :P
> .

The order of options and the offset of fields in private struct have no
effect on ABI. I take these into consideration:
1. Readability. Related options and fields should be put at the same
   place.
2. Memory footprint. Reduce struct padding.

> 
> Jan
> _______________________________________________
> 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".

_______________________________________________
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".