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?
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?
- Andreas
_______________________________________________
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".
