[FFmpeg-devel] [PATCH] CD+G Demuxer & Decoder

Reimar Döffinger Reimar.Doeffinger
Sat Dec 12 01:51:02 CET 2009


On Thu, Dec 10, 2009 at 10:59:23PM -0800, Michael Tison wrote:
> +static int read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    int ret;
> +
> +    ret = av_get_packet(s->pb, pkt, CDG_PACKET_SIZE);
> +    if (ret >= 0 && ret < CDG_PACKET_SIZE) {
> +        av_free_packet(pkt);
> +        ret = AVERROR(EIO);
> +    }
> +    pkt->stream_index = 0;
> +    return ret;

I'd like to restate that I consider this kind of code highly silly, it
1) misuses AVERROR(EIO) for something that probably is not an I/O error
2) it means a single lost byte which might not make a visible difference
   otherwise now drops the whole (last) packet
3) it makes it unnecessarily difficult to test the decoder code for robustness
   and sufficient buffer end checks (no, those in libavformat are not of any
   use, libavcodec is not supposed to be only used with libavformat)
4) and all of that at the cost of additional code and complexity

Or to put it succinctly, I think it is adding code to make it behave worse.



More information about the ffmpeg-devel mailing list