Quoting Andreas Rheinhardt (2022-08-24 18:03:05) > Anton Khirnov: > > The mov demuxer only returns DV audio, video packets are discarded. > > > > It first reads the data to be parsed into a packet. Then both this > > packet and the pointer to its data are passed together to > > avpriv_dv_produce_packet(), which parses the data and partially > > overwrites the packet. This is confusing and potentially dangerous, so > > just pass NULL and avoid pointless packet modification. > > --- > > libavformat/dv.c | 19 +++++++++++-------- > > libavformat/mov.c | 2 +- > > 2 files changed, 12 insertions(+), 9 deletions(-) > > > > diff --git a/libavformat/dv.c b/libavformat/dv.c > > index 303cecf9bb..f88fe62349 100644 > > --- a/libavformat/dv.c > > +++ b/libavformat/dv.c > > @@ -430,14 +430,17 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, > > AVPacket *pkt, > > } > > } > > > > - /* Now it's time to return video packet */ > > - size = dv_extract_video_info(c, buf); > > - pkt->data = buf; > > - pkt->pos = pos; > > - pkt->size = size; > > - pkt->flags |= AV_PKT_FLAG_KEY; > > - pkt->stream_index = c->vst->index; > > - pkt->pts = c->frames; > > + /* return the video packet, if the caller wants it */ > > + if (pkt) { > > + size = dv_extract_video_info(c, buf); > > + > > + pkt->data = buf; > > + pkt->pos = pos; > > + pkt->size = size; > > + pkt->flags |= AV_PKT_FLAG_KEY; > > + pkt->stream_index = c->vst->index; > > + pkt->pts = c->frames; > > + } > > > > c->frames++; > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index 1d8c5b2904..a2b429e52f 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -8768,7 +8768,7 @@ static int mov_read_packet(AVFormatContext *s, > > AVPacket *pkt) > > } > > #if CONFIG_DV_DEMUXER > > if (mov->dv_demux && sc->dv_audio_container) { > > - ret = avpriv_dv_produce_packet(mov->dv_demux, pkt, pkt->data, > > pkt->size, pkt->pos); > > + ret = avpriv_dv_produce_packet(mov->dv_demux, NULL, pkt->data, > > pkt->size, pkt->pos); > > av_packet_unref(pkt); > > if (ret < 0) > > return ret; > > 1. LGTM. > 2. The way mov handles dv audio is very broken: > a) I don't see anything that guarantees that the > samplerate of the inner demuxer and the sample rate reported to the user > (presumably taken from mov structures) coincide. > b) dv_fctx and dv_demux are part of MOVContext, not MOVStreamContext. If > there were multiple MOVStreams with dv-audio, the older dv_fctx and > dv_demux would leak. Do you see anything that guarantees that they will > only be at most one MOVStream for dv-audio?
Not that I can see. I'd say the best thing to do here is fail when a dvaudio stream already exists - trying to support it without having any valid samples seems pointless. I'll send a patch. > c) The former would be easily fixable by moving the fields to > MOVStreamContext. But there is actually another bug: The underlying dv > stream can contain multiple audio streams in this one MOVStream and only > the first of these is forwarded. > https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4671/dir1.tar.bz2 > contains a sample where this actually occurs: The DV stream has two > stereo tracks, only the first of which is forwarded (use -enable_drefs 1 > to be able to play it; the non-forwarded seems to be silent in this > case). Are these actually independent audios or are these stereo pairs > part of a single streams with more than two channels? Cannot answer this. I would ignore this until and unless someone actually comes complaining about this. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".