[FFmpeg-devel] [PATCH]?avcodec_decode_audio3?and?multiple?frames in a packet

Michael Niedermayer michaelni
Mon Oct 5 23:09:52 CEST 2009


On Thu, Sep 17, 2009 at 07:44:28PM +0200, Sascha Sommer wrote:
> Hi,
> 
> On Donnerstag, 17. September 2009, Michael Niedermayer wrote:
> > On Thu, Sep 17, 2009 at 01:37:10PM +0200, Sascha Sommer wrote:
> > > Hi,
> > >
> > > On Mittwoch, 16. September 2009, Michael Niedermayer wrote:
> > > > On Wed, Sep 16, 2009 at 09:24:24PM +0200, Sascha Sommer wrote:
> > > > > Hi,
> > > > >
> > > > > On Mittwoch, 16. September 2009, Michael Niedermayer wrote:
> > > > > > On Wed, Sep 16, 2009 at 05:51:03PM +0200, Sascha Sommer wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Mittwoch, 16. September 2009, Michael Niedermayer wrote:
> > > > > > > > On Wed, Sep 16, 2009 at 02:52:29PM +0200, Sascha Sommer wrote:
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > On Samstag, 12. September 2009, Justin Ruggles wrote:
> > > > > > > > > > Michael Niedermayer wrote:
> > > > > > > > > > > On Fri, Sep 11, 2009 at 12:13:02PM +0200, Sascha Sommer
> > > > > > > > > > > wrote: [...]
> > > > > > > > > > >
> > > > > > > > > > >> Previously a value of 0 meant that no frame was decoded.
> > > > > > > > > > >
> > > > > > > > > > > no
> > > > > > > > > > > you read the docs backward, it says if no frame was
> > > > > > > > > > > decoded return 0 it does not say that 0 means no frames
> > > > > > > > > > > have been decoded, it could equally well mean 0 bytes
> > > > > > > > > > > used
> > > > > > > > > >
> > > > > > > > > > Ah, good.  So, although the current text is technically
> > > > > > > > > > correct if interpreted that way, it is ambiguous.  Why do
> > > > > > > > > > we need to have a 0 return value also possibly mean no
> > > > > > > > > > frames have been decoded?   If frame_size_ptr is set to 0,
> > > > > > > > > > that always means no frames have been decoded, without
> > > > > > > > > > regard to the return value. And a return value of 0 should
> > > > > > > > > > mean zero bytes were used, without regard to what
> > > > > > > > > > frame_size_ptr is set to.  They seem mutually exclusive to
> > > > > > > > > > me...
> > > > > > > > >
> > > > > > > > > I agree. The return value controls the number of input bytes,
> > > > > > > > > frame_size_ptr the number of output bytes. I don't see why 0
> > > > > > > > > needs to be returned when no frame was outputted.
> > > > > > > >
> > > > > > > > what exactly did the decoder then do with the data?
> > > > > > > > and what was that data it did not decode?
> > > > > > >
> > > > > > > It maybe skipped it? For example when the packet contained only
> > > > > > > DSE syntax elements in AAC. I did not check the spec if this can
> > > > > > > happen or for what these Data Stream Elements are actually used
> > > > > > > but as we already found out ffmpeg does
> > > > > > > ? ? ? ? ? ? ? ? avpkt.data += ret;
> > > > > > > ? ? ? ? ? ? ? ? avpkt.size -= ret;
> > > > > > > So this will decode always the same data.
> > > > > >
> > > > > > yes
> > > > > > a packet that is input to a decoder MUST in general contain 1
> > > > > > frame.
> > > > >
> > > > > 1 frame that can be decoded by the decoder, right?
> > > > > But can we under all circumstances know what the decoder produces out
> > > > > of this 1 input frame?
> > > >
> > > > no it can have an error and return a negative value representing the
> > > > type of error
> > >
> > > Maybe we are talking about different things. Let me try to explain again
> > > where I have a problem with understanding the current version of
> > > avcodec_decode_audio3:
> > >
> > > avcodec_decode_audio3 says the following:
> > >  * @return On error a negative value is returned, otherwise the number of
> > > bytes
> > >  * used or zero if no frame could be decompressed.
> > >
> > > How is that written in C (probably the wrong interpretation)?
> > >
> > >         if(error)       //On error a negative value is returned
> > >             return error;
> > >         if(number_of_bytes_used) //otherwise the number of bytes used
> > >             return number_of_bytes_used;
> > >         if(!*data_size) //or zero if no frame could be decompressed
> > >             return 0;
> > >          ... //what to return here?
> > >       I think  this can be reached with CODEC_CAP_DELAY when avpkt->size
> > > == 0 So there should also be return 0
> > >
> > > So this looks to me the same as
> > >         if(error)
> > >             return error;
> > >         if(number_of_bytes_used)
> > >             return number_of_bytes_used;
> > > and the "or zero if no frame could be decompressed." can be removed from
> > > the API description. When number_of_bytes_used is 0 return 0.
> > >
> > > However, judging from the discussion, I think the API description has to
> > > be seen as.
> > >
> > >            if(error)
> > >                return error;
> > >             if(!*data_size)
> > >                return 0;
> > >             return number_of_bytes_used.
> > >
> > > This would mean that 0 is always returned when there is no frame, no
> > > matter what the value of number_of_bytes_used is.
> >
> > there is a difference from "no frame decompressed" and "no frame output"
> > you seem to assume that they are the same in your mail
> >
> 
> Yes this is what I was assuming.
> So am I now right that the above code of doing 
>          if(!*data_size)
>                return 0;
> is wrong and the decoder should really return number_of_bytes_used if it used 
> some bytes no matter if a frame is outputted or not, because it decompressed 
> the frame (by using the bytes)? Only when really no frame, not even a part it 
> is decompressed (and this decompression is not possible without using any 
> bytes) 0 is to be returned? So 0 is to be returned when a frame is only 
> outputted and no new data is consumed from the input packet?

i think that is correct 


> 
> > > ffmpeg.c contains the following code:
> > >
> > >     while (avpkt.size > 0 || (!pkt && ist->next_pts != ist->pts)) {
> > > ...
> > >                 ret = avcodec_decode_audio3(ist->st->codec, samples,
> > > &data_size,
> > >                                             &avpkt);
> > >                 if (ret < 0)
> > >                     goto fail_decode;
> > >                 avpkt.data += ret;
> > >                 avpkt.size -= ret;
> > >                 /* Some bug in mpeg audio decoder gives */
> > >                 /* data_size < 0, it seems they are overflows */
> > >                 if (data_size <= 0) {
> > >                     /* no audio frame */
> > >                     continue;
> > >                 }
> > > ...
> > >
> > > So when there is no frame (ret = 0, data_size <= 0) ffmpeg will call
> > > avcodec_decode_audio3 again with the same avpkt without incrementing
> > > avpkt.data and without decreasing avpkt.size.
> > > Now it depends on the decoder how it handles that.
> > > I think there could be an endless loop when the packet is interpreted the
> > > same
> >
> > data_size=0 means no frame output
> > ret=0 means the decoder did not use any bytes
> > -> the decoder did nothing and at the same time does not consider that an
> > error, yes that leads to an infinite loop if the decoder continues to
> > behave like that.
> > But really its the decoder that refuses to move forward not the application
> > in that case. The decoder could return an error or it could contine to
> > decode.
> >
> > > way as during the first call or there might be a bitstream error when the
> > > decoder expected a follow-up packet.
> > >
> > > If I understand you correctly ("a packet that is input to a decoder MUST
> > > in general contain 1 frame" and "my definition of frame contains a
> > > positive number of samples, not 0 and not
> > > negative"), the previously described case is not allowed to happen.
> > > So why is this not a real error then?
> >
> > well, if it cant happen the point of what if is a if(0)
> >
> > > I always assumed this could happen and this is why I thought that the API
> > > not only needs to be clarified but also should be changed so that it
> > > always returns the number of consumed bytes when there was no error to
> > > stay consistent with the case when a frame is returned and to have an
> > > unambiguous indicator of how many bytes have been used from the packet.
> > > When "no bytes used from the input packet" is the same as "bytes used
> > > from the input packet but no frame returned", the data_size variable
> > > needs to be checked to decide
> >
> > again returned != decoded
> > if you decode nothing that IS pretty much the same as not using any bytes,
> > at least its pretty close ...
> 
> Yes so in 
> 
> "@return On error a negative value is returned, otherwise the number of bytes
> used or zero if no frame could be decompressed."
> 
> "if no frame could be decompressed" is also pretty much the same as not using 
> any bytes? So above sentence could also have been written as "On error a 
> negative value is returned, otherwise the number of bytes
> used"?

probably yes

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091005/b5c6d835/attachment.pgp>



More information about the ffmpeg-devel mailing list