[FFmpeg-devel] [PATCH] Support playing SMV files.

[Michael Niedermayer]

On Sat, Oct 06, 2012 at 11:20:03PM +0000, Ash Hughes wrote:
> Hi,
> 
> Here is an update to the SMV patch. For using one packet for multiple frames, I don't know how to get it to send the same packet multiple times to the decode_frame function. Also, the documentation at http://ffmpeg.org/doxygen/trunk/group__lavf__decoding.html#g4fdb3084415a82e3810de6ee60e46a61 implies that this can't be done anyway, as there is only one video Frame per packet. Perhaps there's a property on AVPacket that I'm missing?
> 
> With the threading issue, I wonder if avcodec_open could be split into a mutexed public part and an unmutexed but private part? This seems messy and potentially dangerous to me though, maybe it would be better to accept the inefficiency for this (rarely used) codec?

calling avcodec_open2() from decode_frame is not an entirely safe act
from a quick look unlocking the mutex and calling it from init seems
safer. Maybe you could add a avcodec_open2_recursiv() that does this


[...]
> @@ -518,6 +527,9 @@
>              avio_rl24(pb);
>              avio_rl24(pb);
>              wav->smv_frames_per_jpeg = avio_rl24(pb);
> +            AV_WL32(vst->codec->extradata, wav->smv_frames_per_jpeg);
> +            wav->smv_cur_pt = 0;
> +            vst->codec->height /= wav->smv_frames_per_jpeg;

division by 0


[...]
> +void smv_image_pnt(uint8_t *dst_data[4], int dst_linesizes[4],
> +                   uint8_t *src_data[4], const int src_linesizes[4],
> +                   enum PixelFormat pix_fmt, int width, int height, int nlines)
> +{
> +    const AVPixFmtDescriptor *desc = &av_pix_fmt_descriptors[pix_fmt];
> +
> +    if (desc->flags & PIX_FMT_HWACCEL)
> +        return;
> +
> +    if (desc->flags & PIX_FMT_PAL ||
> +        desc->flags & PIX_FMT_PSEUDOPAL) {
> +        smv_image_pnt_plane(&dst_data[0], dst_linesizes[0],
> +                            src_data[0], src_linesizes[0],
> +                            width, height, nlines);
> +        /* copy the palette */
> +        dst_data[1] = src_data[1];

is there some case that uses a paletted format ?


> +    } else {
> +        int i, planes_nb = 0;
> +
> +        for (i = 0; i < desc->nb_components; i++)
> +            planes_nb = FFMAX(planes_nb, desc->comp[i].plane + 1);
> +
> +        for (i = 0; i < planes_nb; i++) {
> +            int h = height;
> +            int bwidth = av_image_get_linesize(pix_fmt, width, i);
> +            if (i == 1 || i == 2) {
> +                h= -((-height)>>desc->log2_chroma_h);
> +            }
> +            smv_image_pnt_plane(&dst_data[i], dst_linesizes[i],
> +                                src_data[i], src_linesizes[i],
> +                                bwidth, h, nlines);
> +        }
> +    }
> +}
> +
> +av_cold int ff_smvjpeg_decode_init(AVCodecContext *avctx)
> +{
> +    SMVJpegDecodeContext *s = avctx->priv_data;
> +
> +    s->frames_per_jpeg = AV_RL32(avctx->extradata);
> +    s->jpg.picture_ptr      = &s->picture[0];
> +
> +    avcodec_get_frame_defaults(&s->picture[1]);
> +    avctx->coded_frame = &s->picture[1];
> +    s->codec = avcodec_find_decoder(AV_CODEC_ID_MJPEG);
> +    if (!s->codec)
> +        av_log(avctx, AV_LOG_ERROR, "MJPEG codec not found\n");
> +
> +    s->avctx = avcodec_alloc_context3(s->codec);
> +
> +    return 0;
> +}
> +
> +int ff_smvjpeg_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
> +                            AVPacket *avpkt)
> +{
> +    SMVJpegDecodeContext *s = avctx->priv_data;
> +    int ret = 0;
> +    AVFrame* buf_data = &s->picture[2];
> +    AVFrame* mjpeg_data = &s->picture[0];
> +    AVFrame* output = data;
> +    int u, i;
> +    int cur_frame = avpkt->pts % s->frames_per_jpeg;
> +
> +    /* Are we at the start of a block?
> +       Also call mjpeg if we start on an odd pts and buf_data isn't
> +       allocated yet. */
> +    if (cur_frame == 0 || !buf_data->data[0]) {
> +        /* we can't open or close the codec in init or end so we have
> +           to do it all here. */
> +        if (avcodec_open2(s->avctx, s->codec, NULL) < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "MJPEG codec failed to open\n");
> +            return -1;
> +        } else {
> +            /* Codec opened, create yet another buffer so the avcodec_close
> +               will not wipe it. */
> +            ret = s->codec->decode(s->avctx, mjpeg_data, data_size, avpkt);

this is not correct
you cant mix highlevel (avcodec_open2) and lowlevel (codec->decode())
calls

also make sure that the internal decoder you create gets threads=1
set, running it with multithreading could cause problems

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121010/d2b86681/attachment.asc>