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

[Sascha Sommer]

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?

> > 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"?

Because this will just return 0 when no bytes were used and this is, what I 
understand from your sentence, will also happen when no frame was decoded.

Regards

Sascha