From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id B329C42BEE for ; Wed, 31 Aug 2022 02:39:20 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E290968BB02; Wed, 31 Aug 2022 05:39:17 +0300 (EEST) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 17DE568B406 for ; Wed, 31 Aug 2022 05:39:12 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id A835D240D0E for ; Wed, 31 Aug 2022 04:39:11 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id lLWS8YCPxXIu for ; Wed, 31 Aug 2022 04:39:11 +0200 (CEST) Received: from lain.khirnov.net (lain.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 0450F240D03 for ; Wed, 31 Aug 2022 04:39:11 +0200 (CEST) Received: by lain.khirnov.net (Postfix, from userid 1000) id 0B16D1601B2; Wed, 31 Aug 2022 04:39:11 +0200 (CEST) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: =?utf-8?q?=3CDB6PR0101MB2214EB3700A31480135E0C288F739=40DB6PR01?= =?utf-8?q?01MB2214=2Eeurprd01=2Eprod=2Eexchangelabs=2Ecom=3E?= References: <20220824084318.333-1-anton@khirnov.net> <20220824084318.333-12-anton@khirnov.net> =?utf-8?q?=3CDB6PR0101MB2214EB370?= =?utf-8?q?0A31480135E0C288F739=40DB6PR0101MB2214=2Eeurprd01=2Eprod=2Eexchan?= =?utf-8?q?gelabs=2Ecom=3E?= Mail-Followup-To: FFmpeg development discussions and patches Date: Wed, 31 Aug 2022 04:39:11 +0200 Message-ID: <166191355101.3205.1610958410635781470@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 12/18] lavf/dv: make returning the video packet optional X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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-request@ffmpeg.org with subject "unsubscribe".