[FFmpeg-devel] [PATCH] ROQ encoder: remove redundant messages, reduce constraints

[Michael Niedermayer]

On Mon, Jan 13, 2014 at 10:24:28AM +0000, u-owvm at aetey.se wrote:
> Hello,
> 
> Proposing the following changes:
> 
> Remove redundant messages (about a harmless condition).
> Make Quake compatibility breach to be a mere warning
> and remove the hard constraint on the framerate to make
> the encoder usable/useful in more general scenarios.
> 
> (TODO: constraint/bug compatibility should be a run time option)
> 
> Regards,
> Rl

>  roqvideodec.c |    4 ++--
>  roqvideoenc.c |   15 +++------------
>  2 files changed, 5 insertions(+), 14 deletions(-)
> d137bc4264cb320be998267273f573c8da2a84f2  roq-git.patch
> From 0dffb07e476eb64915c9dc5363b6faa14e6105d7 Mon Sep 17 00:00:00 2001
> From: Rl <addr-see-the-website at aetey.se>
> Date: Sun, 12 Jan 2014 18:54:12 +0100
> Subject: [PATCH] Remove redundant messages (about a harmless condition).
>  Make Quake compatibility breach to be a mere warning
>  and remove the hard constraint on the framerate to make
>  the encoder usable/useful in more general scenarios.
> 
> ---
>  libavcodec/roqvideodec.c |    4 ++--
>  libavcodec/roqvideoenc.c |   15 +++------------
>  2 files changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/libavcodec/roqvideodec.c b/libavcodec/roqvideodec.c
> index 8e7dffe..df2a977 100644
> --- a/libavcodec/roqvideodec.c
> +++ b/libavcodec/roqvideodec.c
> @@ -79,7 +79,7 @@ static void roqvideo_decode_frame(RoqContext *ri)
>          for (yp = ypos; yp < ypos + 16; yp += 8)
>              for (xp = xpos; xp < xpos + 16; xp += 8) {
>                  if (bytestream2_tell(&ri->gb) >= chunk_start + chunk_size) {
> -                    av_log(ri->avctx, AV_LOG_ERROR, "Input buffer too small\n");
> +//                    av_log(ri->avctx, AV_LOG_ERROR, "Input buffer too small\n");
>                      return;
>                  }
>                  if (vqflg_pos < 0) {
> @@ -114,7 +114,7 @@ static void roqvideo_decode_frame(RoqContext *ri)
>                          if(k & 0x02) y += 4;
>  
>                          if (bytestream2_tell(&ri->gb) >= chunk_start + chunk_size) {
> -                            av_log(ri->avctx, AV_LOG_ERROR, "Input buffer too small\n");
> +//                            av_log(ri->avctx, AV_LOG_ERROR, "Input buffer too small\n");
>                              return;
>                          }
>                          if (vqflg_pos < 0) {

do you have roq files which cause these messages to be printed but
which decode correctly ?



> diff --git a/libavcodec/roqvideoenc.c b/libavcodec/roqvideoenc.c
> index 37bd8d5..618e5ab 100644
> --- a/libavcodec/roqvideoenc.c
> +++ b/libavcodec/roqvideoenc.c
> @@ -903,18 +903,9 @@ static void roq_encode_video(RoqContext *enc)
>      /* Quake 3 can't handle chunks bigger than 65535 bytes */
>      if (tempData->mainChunkSize/8 > 65535) {
>          av_log(enc->avctx, AV_LOG_ERROR,
> -               "Warning, generated a frame too big (%d > 65535), "
> -               "try using a smaller qscale value.\n",
> +               "Warning, generated a frame too big for Quake (%d > 65535), "
> +               "for compatibility you may wish to increase qscale value.\n",
>                 tempData->mainChunkSize/8);
> -        enc->lambda *= 1.5;
> -        tempData->mainChunkSize = 0;
> -        memset(tempData->used_option, 0, sizeof(tempData->used_option));
> -        memset(tempData->codebooks.usedCB4, 0,
> -               sizeof(tempData->codebooks.usedCB4));
> -        memset(tempData->codebooks.usedCB2, 0,
> -               sizeof(tempData->codebooks.usedCB2));
> -
> -        goto retry_encode;
>      }
>  
>      remap_codebooks(enc, tempData);

a quake compatibility option like you suggest in the TODO would be
much nicer than requiring the user to globally reduce quality to be
compatibile with quake




> @@ -1082,7 +1073,7 @@ AVCodec ff_roq_encoder = {
>      .init                 = roq_encode_init,
>      .encode2              = roq_encode_frame,
>      .close                = roq_encode_end,
> -    .supported_framerates = (const AVRational[]){ {30,1}, {0,0} },
> +/*     .supported_framerates = (const AVRational[]){ {30,1}, {0,0} }, */
>      .pix_fmts             = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV444P,
>                                                          AV_PIX_FMT_NONE },
>  };

if its unneeded  then it should be completely removed not just
commented out
or maybe if its needed for the official decoders then a new
AVCodec struct could be added without this restriction
or AVCodecContext.profile could be used to select which restrictions
apply

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20140113/c27f3d24/attachment.asc>