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

[Justin Ruggles]

Sascha Sommer wrote:

> Hi,
> 
> attached patch extends the documentation of avcodec_decode_audio3 a bit.
> Previously, the case when multiple frames are stored in a single packet was 
> not documented very well. I described the current behaviour of ffmpeg. That 
> is:
>                 avpkt.data += ret;
>                 avpkt.size -= ret;
> is done until the packet is fully decoded.
> This also makes it possible to return 0 until the last frame in a packet is 
> decoded. This is currently done for wmapro.
> 
> What really changes the API is the change of the return value so maybe this 
> should become avcodec_decode_audio4.
> Previously a value of 0 meant that no frame was decoded.
> I do not like this implication because
> 
> 1. This can already be found out by checking *frame_size_ptr what should be 0
>      then.
> 2. Consider a hypothetical audio codec that stores two 4-bit frames in a
>     8-bit packet. The description of the function says that decoding stops
>     after the first frame but what to return then?
>     return 0 means that no frame has been decoded => wrong
>     return 1 will abort the decoding after the first frame => wrong
>     

> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h	(revision 19753)
> +++ libavcodec/avcodec.h	(working copy)
> @@ -3202,7 +3202,16 @@
>  /**
>   * Decodes the audio frame of size avpkt->size from avpkt->data into samples.
>   * Some decoders may support multiple frames in a single AVPacket, such
> - * decoders would then just decode the first frame.
> + * decoders would then just decode the first frame. If the same AVPacket
> + * is passed in again, the second frame is returned and so on.


What exactly is meant by "same AVPacket"?  If the pointer is the same,
but the data inside it has changed is that the same packet, or
vice-versa?  If a "different" AVPacket is passed before the previous one
has had all data consumed, what happens?

> + * The AVPacket is fully decoded and the decoder is ready for the next
> + * AVPacket when the number of returned bytes is equal to avpkt->size.
> + *
> + * If the AVPacket is not fully decoded yet, avpkt->data needs to be
> + * incremented by the number of returned bytes and avpkt->size needs
> + * to be decremented by the number of returned bytes before the function
> + * is called again to get the next frame in the AVPacket.


I don't agree with this change.  Requiring specific external changes to
the state of a passed parameter based on the return value then assuming
these changes in the next call of the function seems counter-intuitive
and somewhat unstable.

Also, the application may want to discard the rest of the packet after a
partial decode.  How would the decoder know this?  Would this be
disallowed?  Would it be allowed but be considered seeking and require
an AVCodec.flush() before the next call?

> + *
>   * If no frame
>   * could be decompressed, frame_size_ptr is zero. Otherwise, it is the
>   * decompressed frame size in bytes.
> @@ -3234,7 +3243,7 @@
>   *            data and size, some decoders might in addition need other fields.
>   *            All decoders are designed to use the least fields possible though.
>   * @return On error a negative value is returned, otherwise the number of bytes
> - * used or zero if no frame could be decompressed.
> + * used.


Independent of your other proposal, I do agree with this change.  It
would also be more consistent with the use of CODEC_CAP_DELAY, where no
data is consumed but the decoder can output a frame.


Cheers,
Justin