[FFmpeg-devel] [PATCH v2 4/8] ffmpeg: remove sub-frame warning

[Michael Niedermayer]

On Wed, Mar 23, 2016 at 09:51:46PM +0100, wm4 wrote:
> On Wed, 23 Mar 2016 21:36:35 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Wed, Mar 23, 2016 at 08:44:41PM +0100, wm4 wrote:
> > > On Wed, 23 Mar 2016 18:37:25 +0100
> > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > >   
> > > > On Wed, Mar 23, 2016 at 06:06:37PM +0100, wm4 wrote:  
> > > > > On Wed, 23 Mar 2016 17:51:11 +0100
> > > > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > > >     
> > > > > > On Wed, Mar 23, 2016 at 02:02:11PM +0100, wm4 wrote:    
> > > > > > > It's not practical to keep this with the new decode API.
> > > > > > > ---
> > > > > > >  ffmpeg.c | 7 -------
> > > > > > >  ffmpeg.h | 1 -
> > > > > > >  2 files changed, 8 deletions(-)      
> > > > > > 
> > > > > > its not practical in ffmpeg.c but libavcodec should be able to easily
> > > > > > check that a decoder which doesnt declare AV_CODEC_CAP_SUBFRAMES
> > > > > > doesnt decode "subframes"
> > > > > > Can you move this check into libavcodec ?
> > > > > > i think otherwise nothing would be checking for missing
> > > > > > AV_CODEC_CAP_SUBFRAMES anymore
> > > > > >     
> > > > > 
> > > > > What's the point of this check?    
> > > > 
> > > > to keep track of / detect the cases that put multiple decodable frames
> > > > in a packet.
> > > > 
> > > > Whats the point of that?
> > > > there where several IIRC
> > > > one is that when too many frames are put in a packet 
> > > > latency increases, another is that seeking granularity is worse
> > > > (if its not even one packet for the whole file ...)  
> > > 
> > > It's true that too many frames in a packet isn't ideal, but that's not
> > > what the code checks.
> > > 
> > > It checks if an audio decoder not marked with AV_CODEC_CAP_SUBFRAMES
> > > consumes partial packets.  
> > 
> > yes, but a check that checks "if a decoder not marked with
> > AV_CODEC_CAP_SUBFRAMES consumes partial packets". Is a simple and
> > zero overhead way of detecting some (not all) cases where there are
> > multiple frames in a packet. One cant look at a sequence of bytes
> > that could be any arbitrary format/codec and say
> > "thats more than 1 frame" it requires codec specific code,
> > the decoders already do what is needed for some cases, for the others
> > there is (please correct me if iam wrong which might be) no easy
> > way except maybe running the parser if one exists over it but that
> > would not be zero overhead
> > 
> > 
> > > That might be useful as debug check in
> > > libavcodec or so, or by properly reviewing patches for new decoders.  
> > 
> > Iam not sure if i understand what you mean exactly but this somehow
> > sounds like an implication that people would not review patches
> > properly.
> > Thats a serious accusation if thats what was meant. Either there is
> > a problem then it should be pointed to very specifically so it can be
> > solved or such implications shouldnt be made at all.
> 
> Well, I'm not sure what else this check is useful for. A new audio
> decoder will need explicit code to handle multiframe audio by returning
> the exact number of bytes parsed, instead of e.g.
> "return avpkt->size;". So it should be pretty obvious whether a decoder
> does this?

yes, though there may be corner cases where it has to do that
and it might be more robust.



> 
> Anyway, I could move this check to avcodec_decode_audio4(), would that
> be ok?

yes and thanks!


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160323/a0dc84d2/attachment.sig>