| Message ID | 20180314062445.89909-7-rodger.combs@gmail.com |
|---|---|
| State | Withdrawn, archived |
| Headers | show |
On 3/14/2018 3:24 AM, Rodger Combs wrote: > This is assumed not to break API because it's already true (see e.g. > matroskaenc handling of new AAC extradata) That's an API violation from the muxer's part instead. The correct approach is to fix the violation, not adapt the API around it. > --- > libavformat/avformat.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index 9e87d6cdac..5f0ebfc114 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -1006,7 +1006,8 @@ typedef struct AVStream { > * > * - demuxing: filled by libavformat on stream creation or in > * avformat_find_stream_info() > - * - muxing: filled by the caller before avformat_write_header() > + * - muxing: filled by the caller before avformat_write_header(); > + * - may be modified by libavformat afterwards Personally I'd rather fix the current muxer-level violations and if needed store any such changes in a separate struct in AVStreamInternal instead when dealing with generic code (Namely patch 9/10 from this set). But if others are fine with introducing this definition change then i wont block it. > */ > AVCodecParameters *codecpar; > >
On Wed, Mar 14, 2018 at 01:24:42AM -0500, Rodger Combs wrote: > --- > libavformat/avformat.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index 9e87d6cdac..5f0ebfc114 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -1006,7 +1006,8 @@ typedef struct AVStream { > * > * - demuxing: filled by libavformat on stream creation or in > * avformat_find_stream_info() > - * - muxing: filled by the caller before avformat_write_header() > + * - muxing: filled by the caller before avformat_write_header(); > + * - may be modified by libavformat afterwards > */ a generic "anything can change" is not really practical a user app would have to check every field of codecpar and possibly insert video scalers if resoluton changed, or all kinds of other filters and even different encoders if codec_id was adjusted ... [...]
On Fri, Mar 16, 2018 at 12:29 AM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Mar 14, 2018 at 01:24:42AM -0500, Rodger Combs wrote: >> --- >> libavformat/avformat.h | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h >> index 9e87d6cdac..5f0ebfc114 100644 >> --- a/libavformat/avformat.h >> +++ b/libavformat/avformat.h >> @@ -1006,7 +1006,8 @@ typedef struct AVStream { >> * >> * - demuxing: filled by libavformat on stream creation or in >> * avformat_find_stream_info() >> - * - muxing: filled by the caller before avformat_write_header() >> + * - muxing: filled by the caller before avformat_write_header(); >> + * - may be modified by libavformat afterwards >> */ > > a generic "anything can change" is not really practical > a user app would have to check every field of codecpar and possibly insert > video scalers if resoluton changed, or all kinds of other filters and even > different encoders if codec_id was adjusted ... > I generally don't like this either. codecpar describes the codec as given to the muxer by the user. The muxer has no business changing things around in it. If it can derive better values somehow, let it store it internally somewhere. - Hendrik
Could we just declare that lavf can update extradata (and nothing else) if it gets packets with new-extradata side-data? If not, I suppose we could either add something to AVStreamInternal, or do something internal in check_bitstream (and update movenc and matroskaenc, as both exhibit this behavior right now). > On Mar 15, 2018, at 20:04, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > > On Fri, Mar 16, 2018 at 12:29 AM, Michael Niedermayer > <michael@niedermayer.cc <mailto:michael@niedermayer.cc>> wrote: >> On Wed, Mar 14, 2018 at 01:24:42AM -0500, Rodger Combs wrote: >>> --- >>> libavformat/avformat.h | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h >>> index 9e87d6cdac..5f0ebfc114 100644 >>> --- a/libavformat/avformat.h >>> +++ b/libavformat/avformat.h >>> @@ -1006,7 +1006,8 @@ typedef struct AVStream { >>> * >>> * - demuxing: filled by libavformat on stream creation or in >>> * avformat_find_stream_info() >>> - * - muxing: filled by the caller before avformat_write_header() >>> + * - muxing: filled by the caller before avformat_write_header(); >>> + * - may be modified by libavformat afterwards >>> */ >> >> a generic "anything can change" is not really practical >> a user app would have to check every field of codecpar and possibly insert >> video scalers if resoluton changed, or all kinds of other filters and even >> different encoders if codec_id was adjusted ... >> > > I generally don't like this either. codecpar describes the codec as > given to the muxer by the user. The muxer has no business changing > things around in it. > If it can derive better values somehow, let it store it internally somewhere. > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 9e87d6cdac..5f0ebfc114 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1006,7 +1006,8 @@ typedef struct AVStream { * * - demuxing: filled by libavformat on stream creation or in * avformat_find_stream_info() - * - muxing: filled by the caller before avformat_write_header() + * - muxing: filled by the caller before avformat_write_header(); + * - may be modified by libavformat afterwards */ AVCodecParameters *codecpar;