[FFmpeg-devel] [PATCH] 8088flex TMV demuxer and decoder

[Michael Niedermayer]

On Wed, Apr 22, 2009 at 02:33:42PM -0500, Daniel Verkamp wrote:
> On Wed, Apr 22, 2009 at 2:17 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Apr 22, 2009 at 09:42:29AM -0500, Daniel Verkamp wrote:
> > [...]
> >
> >> + ? ?for (y = 0; y < char_rows; y++) {
> >> + ? ? ? ?for (x = 0; x < char_cols; x++) {
> >> + ? ? ? ? ? ?c ?= *src++ * 8;
> >> + ? ? ? ? ? ?bg = *src ?>> 4;
> >> + ? ? ? ? ? ?fg = *src++ & 0xF;
> >> +
> >> + ? ? ? ? ? ?dst_char = dst + x * 8;
> >> + ? ? ? ? ? ?for (char_y = 0; char_y < 8; char_y++) {
> >> + ? ? ? ? ? ? ? ?for (mask = 0x80; mask; mask >>= 1) {
> >> + ? ? ? ? ? ? ? ? ? ?*dst_char++ = ff_cga_font[c + char_y] & mask ? fg : bg;
> >> + ? ? ? ? ? ? ? ?}
> >> + ? ? ? ? ? ? ? ?dst_char += tmv->pic.linesize[0] - 8;
> >> + ? ? ? ? ? ?}
> >> + ? ? ? ?}
> >
> > compression wise this is not wise ...
> > you could store a bit for each of the 80x25 chars and only if its
> > 1 store color+char oterwise copy from the same location of the previous frame
> > similarly the color could be reused from the last frame and just the char
> > updated
> >
> > also if you sort the 256 chars based on their amount of foreground/background
> > pixels then chars might be nummerically correlated and compression of changed
> > chars might be possible by huffman coded differences.
> >
> > speedwise it can of course be done faster by building more than 1 pixel at a
> > time but i guess theres no point, its better if it stays simple.
> >
> 
> If this is a critique of the format itself, I didn't design it. :)

yes, theres alot that could be improved :)


> Or do you mean I should change the decoder somehow?
> 
> >
> > [...]
> >> +static int tmv_read_header(AVFormatContext *s, AVFormatParameters *ap)
> >> +{
> >> + ? ?TMVContext *tmv ? = s->priv_data;
> >> + ? ?ByteIOContext *pb = s->pb;
> >> + ? ?AVStream *vst, *ast;
> >> +
> >> + ? ?if (get_le32(pb) != TMV_TAG)
> >> + ? ? ? ?return -1;
> >> +
> >> + ? ?if (!(vst = av_new_stream(s, 0)))
> >> + ? ? ? ?return AVERROR(ENOMEM);
> >> +
> >> + ? ?if (!(ast = av_new_stream(s, 0)))
> >> + ? ? ? ?return AVERROR(ENOMEM);
> >> +
> >> + ? ?ast->codec->sample_rate = get_le16(pb);
> >> + ? ?tmv->audio_chunk_size ? = get_le16(pb);
> >> + ? ?tmv->comp_method ? ? ? ?= get_byte(pb);
> >> + ? ?if (tmv->comp_method) {
> >> + ? ? ? ?av_log(s, AV_LOG_ERROR, "unsupported compression method %d\n",
> >> + ? ? ? ? ? ? ? tmv->comp_method);
> >> + ? ? ? ?return -1;
> >> + ? ?}
> >> +
> >> + ? ?tmv->char_cols = get_byte(pb);
> >> + ? ?tmv->char_rows = get_byte(pb);
> >> +
> >> + ? ?tmv->features ?= get_byte(pb);
> >> + ? ?if (tmv->features & ~(TMV_PADDING | TMV_STEREO)) {
> >> + ? ? ? ?av_log(s, AV_LOG_ERROR, "unsupported features 0x%02x\n",
> >> + ? ? ? ? ? ? ? tmv->features & ~(TMV_PADDING | TMV_STEREO));
> >> + ? ? ? ?return -1;
> >> + ? ?}
> >> +
> >> + ? ?ast->codec->codec_type ? ? ? ? ? ?= CODEC_TYPE_AUDIO;
> >> + ? ?ast->codec->codec_id ? ? ? ? ? ? ?= CODEC_ID_PCM_U8;
> >> + ? ?ast->codec->sample_fmt ? ? ? ? ? ?= SAMPLE_FMT_U8;
> >> + ? ?ast->codec->channels ? ? ? ? ? ? ?= tmv->features & TMV_STEREO ? 2 : 1;
> >> + ? ?ast->codec->bits_per_coded_sample = 8;
> >> + ? ?ast->codec->bit_rate ? ? ? ? ? ? ?= ast->codec->sample_rate *
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ast->codec->bits_per_coded_sample;
> >> + ? ?tmv->pts_inc[1] ? ? ? ? ? ? ? ? ? = tmv->audio_chunk_size;
> >> + ? ?av_set_pts_info(ast, 32, 1, ast->codec->sample_rate);
> >> +
> >> + ? ?vst->codec->codec_type = CODEC_TYPE_VIDEO;
> >> + ? ?vst->codec->codec_id ? = CODEC_ID_TMV;
> >> + ? ?vst->codec->pix_fmt ? ?= PIX_FMT_PAL8;
> >> + ? ?vst->codec->width ? ? ?= tmv->char_cols * 8;
> >> + ? ?vst->codec->height ? ? = tmv->char_rows * 8;
> >> + ? ?tmv->pts_inc[0] ? ? ? ?= 1;
> >> + ? ?av_set_pts_info(vst, 32, tmv->audio_chunk_size,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ast->codec->sample_rate * ast->codec->channels);
> >
> > if you set av_set_pts_info() so each contain 1 frame then you should not
> > need to set pts of packets, lavf can add 1 as well
> >
> 
> Ok, removed this.  It seems I don't need to set pts for PCM audio
> either, correct?

yes

[...]
> +static av_cold int tmv_decode_init(AVCodecContext *avctx)
> +{
> +    TMVContext *tmv = avctx->priv_data;
> +
> +    tmv->pic.data[0] = NULL;
> +
> +    return 0;
> +}

seems useless

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- 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/20090422/c1c73583/attachment.pgp>