[FFmpeg-devel] Patch to fix qtrle endian issue with rgb555 pixel format

[Michael Niedermayer]

On Sat, Feb 27, 2010 at 07:16:35PM -0800, Mo DeJong wrote:
> The attached patch fixes encoding to a rle555 (thousands of colors)
> pixel format with the qtrle video codec.
> 
> $ ffmpeg -y -i m1b1BPR_QTPRO_16bpp.mov -vcodec qtrle -pix_fmt rgb555
> m1b1BPR_FFMPEG_16bpp_reexport.mov
> 
>   Duration: 00:00:01.50, start: 0.000000, bitrate: 4 kb/s
>     Stream #0.0(eng): Video: qtrle, rgb555le, 1x1, 0 kb/s, 2 fps, 2
> tbr, 1k tbn, 1k tbc
> Output #0, mov, to 'm1b1BPR_FFMPEG_16bpp_reexport.mov':
>   Metadata:
>     encoder         : Lavf52.54.0
>     Stream #0.0(eng): Video: qtrle, rgb555le, 1x1, q=2-31, 200 kb/s, 2
> tbn, 2 tbc
> 
> The problem was that pixels were being written in native endian
> (little) format when they should have been written as big endian.
> 
> cheers
> Mo DeJong

>  qtrleenc.c |   29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 8ddbabc9567475af8bba0ddaadf1c25894db95ac  qtrle_rgb555.patch
> diff --git a/libavcodec/qtrleenc.c b/libavcodec/qtrleenc.c
> index eb275d4..b39a95b 100644
> --- a/libavcodec/qtrleenc.c
> +++ b/libavcodec/qtrleenc.c
> @@ -58,6 +58,25 @@ typedef struct QtrleEncContext {
>      uint8_t* skip_table;
>  } QtrleEncContext;
>  
> +static void qtrle_write_pixels(uint8_t **b, const uint8_t *src, unsigned int size, int pixel_size) {
> +    if (pixel_size == 2) {
> +        /* writing a 16 bit rgb555 has to take endian issues into account, convert to BE on LE machines */
> +        uint16_t inPixel;
> +        uint16_t outPixel;
> +        const int pixel_count = size / 2;
> +        uint16_t *outBuffer = av_mallocz(size);
> +        for (int i = 0; i < pixel_count; i++) {
> +            inPixel = *((((const uint16_t*) src) + i));
> +            AV_WB16(&outPixel, inPixel);
> +            *outBuffer = outPixel;
> +        }
> +        bytestream_put_buffer(b, (const uint8_t *)outBuffer, size);
> +        av_free(outBuffer);
> +    } else {
> +        bytestream_put_buffer(b, src, size);            
> +    }
> +}
> +
>  static av_cold int qtrle_encode_init(AVCodecContext *avctx)
>  {
>      QtrleEncContext *s = avctx->priv_data;
> @@ -68,9 +87,9 @@ static av_cold int qtrle_encode_init(AVCodecContext *avctx)
>      s->avctx=avctx;
>  
>      switch (avctx->pix_fmt) {
> -/*    case PIX_FMT_RGB555:
> +    case PIX_FMT_RGB555:
>          s->pixel_size = 2;
> -        break;*/
> +        break;
>      case PIX_FMT_RGB24:
>          s->pixel_size = 3;
>          break;
> @@ -223,12 +242,12 @@ static void qtrle_encode_line(QtrleEncContext *s, AVFrame *p, int line, uint8_t
>          }
>          else if (rlecode > 0) {
>              /* bulk copy */
> -            bytestream_put_buffer(buf, this_line + i*s->pixel_size, rlecode*s->pixel_size);
> +            qtrle_write_pixels(buf, this_line + i*s->pixel_size, rlecode*s->pixel_size, s->pixel_size);
>              i += rlecode;
>          }
>          else {
>              /* repeat the bits */
> -            bytestream_put_buffer(buf, this_line + i*s->pixel_size, s->pixel_size);
> +            qtrle_write_pixels(buf, this_line + i*s->pixel_size, s->pixel_size, s->pixel_size);
>              i -= rlecode;
>          }
>      }
> @@ -328,6 +347,6 @@ AVCodec qtrle_encoder = {
>      qtrle_encode_init,
>      qtrle_encode_frame,
>      qtrle_encode_end,
> -    .pix_fmts = (const enum PixelFormat[]){PIX_FMT_RGB24, PIX_FMT_ARGB, PIX_FMT_NONE},
> +    .pix_fmts = (const enum PixelFormat[]){PIX_FMT_RGB24, PIX_FMT_ARGB, PIX_FMT_RGB555, PIX_FMT_NONE},

why not:

-PIX_FMT_RGB555
+PIX_FMT_RGB555BE

?



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

Incandescent light bulbs waste a lot of energy as heat so the EU forbids them.
Their replacement, compact fluorescent lamps, much more expensive, dont fit in
many old lamps, flicker, contain toxic mercury, produce a fraction of the light
that is claimed and in a unnatural spectrum rendering colors different than
in natural light. Ah and we now need to turn the heaters up more in winter to
compensate the lower wasted heat. Who wins? Not the environment, thats for sure
-------------- 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/20100228/77565ce9/attachment.pgp>