On Sun, 4 May 2014, Anton Khirnov wrote:
On Sun, 4 May 2014 12:24:48 +0300 (EEST), Martin Storsjö <[email protected]>
wrote:
On Sat, 3 May 2014, Anton Khirnov wrote:
---
libavformat/mux.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/libavformat/mux.c b/libavformat/mux.c
index dd88225..7f67c4d 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -407,6 +407,9 @@ static int compute_pkt_fields2(AVFormatContext *s, AVStream
*st, AVPacket *pkt)
static int write_packet(AVFormatContext *s, AVPacket *pkt)
{
+ AVStream *st = s->streams[pkt->stream_index];
+ uint8_t *new_extradata;
+ int extradata_size;
int ret;
if (!(s->oformat->flags & (AVFMT_TS_NEGATIVE | AVFMT_NOTIMESTAMPS))) {
AVRational time_base = s->streams[pkt->stream_index]->time_base;
@@ -424,6 +427,21 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
if (pkt->pts != AV_NOPTS_VALUE)
pkt->pts += offset;
}
+
+ new_extradata = av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
+ &extradata_size);
+ if (new_extradata) {
+ av_freep(&st->codec->extradata);
+ st->codec->extradata_size = 0;
+
+ st->codec->extradata = av_mallocz(extradata_size +
FF_INPUT_BUFFER_PADDING_SIZE);
+ if (!st->codec->extradata)
+ return AVERROR(ENOMEM);
+
+ memcpy(st->codec->extradata, new_extradata, extradata_size);
+ st->codec->extradata_size = extradata_size;
+ }
+
ret = s->oformat->write_packet(s, pkt);
if (s->pb && ret >= 0 && s->flags & AVFMT_FLAG_FLUSH_PACKETS)
--
1.7.10.4
I didn't read most of the rest of the patchset yet - is this used
somewhere or just a suggestion on how to fix general behaviour?
Yes, 08/10 uses it.
The practical problem I'm solving here is that the flac extradata contains
information that is only known at the end of encoding -- md5 of the whole
content and total number of samples.
Ok, thanks - that explains the reasoning behind this. Is there any
extradata provided by the encoder at startup at all (which is then later
changed), or is it only set at the end?
The way our encoder does this currently is that on flush it just updates the
extradata, but does not return any packets. Then the flac muxer overwrites this
updated extradata in the header.
Obviously, this will only work if:
- the stream codec context is the encoding context (which I think we all agree
is evil and needs to be eliminated)
Yes, that requirement should be eliminated.
- or if the new extradata is somehow sent to the muxer. From currently available
mechanisms, i see only two ways for this:
* we use the side data, as is done in this patchset
* we mandate that the codec context needs to be updated manually (or perhaps
though avcodec_copy_context()) by the caller. But this brings other kinds
of breakage.
Yes, expecting the caller to forward such things isn't ideal either. Side
data probably is the best solution for these cases.
There are a number of similar kinda grayzone cases where different muxers
work better or worse with different encoders. For say h264 encoding, a
less well-behaved encoder might not provide extradata after the
avcodec_open2 call, only after encoding the first packet. A muxer which
only uses the extradata in the trailer, such as the mov muxer, will work
just fine with this, but the flv muxer (which writes the extradata only
when the muxer is opened) won't. This obviously only works if the encoding
is done in the stream context, or if the caller takes care of passing on
extradata if it is provided late.
If one wants to support changes in extradata, the muxer itself needs to be
made aware of the change in one way or another. Doing this here doesn't
stop the muxer from reading the same side data and doing something with
it, of course, but a muxer unaware of the changes will only see the latest
extradata, not the first. Which one of them is the less wrong one depends
on the actual case of course, but neither of them is really right if you
want to handle the changes (e.g. write them inline in the stream at the
right place).
Secondly; this overwrites data in AVFormatContext->streams[x]->codec,
which muxers didn't use to touch at all before. If the user uses this as
the actual context for the encoders (which you ideally shouldn't, but
which I bet a lot do, which I'd prefer we didn't break), we'd be
clobbering data which only the encoder is supposed to set. OTOH I don't
think anything bad would happen by doing it in practice though.
I guess I can avoid those problems for now, by dropping this patch and making
the flac muxer explicitly check for updated extradata in the side data.
Or if you see a nicer way, I'm listening.
No, dropping this patch and doing the same check within the flac muxer
probably is better.
As for why this patch isn't ideal - the use case I've got in mind is for
h264 with SPS/PPS in extradata (such as in flv), where e.g. a resolution
change would mean new extradata. For that scenario, this patch doesn't
help at all and is mostly just misleading.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
