[FFmpeg-devel] movenc: add write_btrt option
"zhilizhao(赵志立)"
quinkblack at foxmail.com
Fri Apr 8 05:47:15 EEST 2022
> On Apr 8, 2022, at 4:36 AM, Jan Ekström <jeebjp at gmail.com> wrote:
>
> On Thu, Apr 7, 2022 at 11:42 AM Eran Kornblau <eran.kornblau at kaltura.com> wrote:
>>
>>>
>>>
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of "zhilizhao(???)"
>>> Sent: Wednesday, 6 April 2022 11:46
>>> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
>>> Subject: Re: [FFmpeg-devel] movenc: add write_btrt option
>>>
>>>> On Apr 3, 2022, at 1:07 PM, Eran Kornblau <eran.kornblau at 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&data=
>>>> 04%7C01%7C%7Cb9907958f97048f5645708da17a9f3c8%7C0c503748de3f4e2597e268
>>>> 19d53a42b6%7C1%7C0%7C637848315958196733%7CUnknown%7CTWFpbGZsb3d8eyJWIj
>>>> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am
>>>> p;sdata=flQc21b5EVWTy%2Bkmw%2FncIWzdLqUxY5XFislPRs5Ij6o%3D&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 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