[FFmpeg-devel] [PATCH 0/4] User controllable padding

[Michael Niedermayer]

On Wed, Jan 01, 2014 at 07:24:00PM +0100, James Darnley wrote:
> On 2014-01-01 13:46, Michael Niedermayer wrote:
> > On Wed, Jan 01, 2014 at 01:01:24PM +0100, James Darnley wrote:
> >> On 2013-12-30 20:16, James Almer wrote:
> >>> -1 for default value, 0 for no padding. Muxers can and should clip the value if needed.
> >>> Ideally look up the corresponding limit for both flac and id3v2.
> >>
> >>> You need to keep the default of 8192 if there's no user provided value.
> >>> Same with the value in the id3v2 patch.
> >>
> > 
> >> Why should there be a "default value" value?  Why would different
> >> formats have different defaults for a feature intended to do the same
> >> thing on them all?
> > 
> > different formats have different requirements. different
> > recommanditions, differnt constraints due to compatibility with
> > various software.
> > Having per format defaults is the easy solution.
> > If you want to change the default padding used by some format then
> > theres the question if that breaks any software or hardware that reads
> > that format. This would have to be tested ...
> > 
> > but its a bit worse then that, even if for the current formats
> > theres a common default that would work for all, a format added
> > in the future might have different requirements, like the padding
> > being a multiple of 12345
> > 
> > also it might make sense to instead use private options for each
> > muxer that supports this, as that way it would be clear which
> > muxers support it and what is the valid range of padding for each
> > format, as well as the default, its not clear if this or a common
> > field in AVFormatContext is better
> 
> I'm not going to be adding something that prints out half a dozen more
> lines (at present) in the help.  The option is supposed represent the
> same thing for many formats.  We don't have separate -metadata options
> for each muxer to tell the user what is possible and what is not.


> 
> > for AVFormatContext fields you need accessors though (for ABI compat),
> > see MAKE_ACCESSORS()
> 
> I have no idea what you mean with this.

MAKE_ACCESSORS() is required for all fields that are accessed from
outside libavformat and are not in the fork
That is to ensure ABI compatibility.


> Searching shows that
> MAKE_ACCESSORS() gives get and set functions but this is only used for a
> few fields.  What did you want?

Please add set/get accessors for the field you add. Also the doxy
should contain a note that says these accessors should be used for
accesses from outside libavformat. You can copy and paste that note
from the doxy of some other field

Thanks & happy new year

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140101/1acddfec/attachment.asc>