[FFmpeg-devel] [PATCH] avcodec/ffv1: Support for RGBA64 and GBRAP16

[Michael Niedermayer]

On Mon, Feb 05, 2018 at 10:05:50PM +0100, Jerome Martinez wrote:
> On 04/02/2018 15:00, Michael Niedermayer wrote:
> >
> >>diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c
> >>index b7eea0dd70..2763082540 100644
> >>--- a/libavcodec/ffv1enc_template.c
> >>+++ b/libavcodec/ffv1enc_template.c
> >>@@ -122,8 +122,8 @@ static av_always_inline int RENAME(encode_line)(FFV1Context *s, int w,
> >>      return 0;
> >>  }
> >>-static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[3],
> >>-                                    int w, int h, const int stride[3])
> >>+static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[4],
> >>+                                    int w, int h, const int stride[4])
> >>  {
> >>      int x, y, p, i;
> >>      const int ring_size = s->context_model ? 3 : 2;
> >>@@ -152,14 +152,18 @@ static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[3],
> >>                  r = (v >> 16) & 0xFF;
> >>                  a =  v >> 24;
> >>              } else if (packed) {
> >>-                const uint16_t *p = ((const uint16_t*)(src[0] + x*6 + stride[0]*y));
> >>+                const uint16_t *p = ((const uint16_t*)(src[0] + x*(3 + s->transparency)*2 + stride[0]*y));
> >>                  r = p[0];
> >>                  g = p[1];
> >>                  b = p[2];
> >>+                if (s->transparency)
> >transparency should possibly be moved into a local variable
> 
> Done, actually both s->transparency and (3 + s->transparency)*2
> 
> >
> >
> >>+                  a = p[3];
> >>              } else if (sizeof(TYPE) == 4) {
> >>                  g = *((const uint16_t *)(src[0] + x*2 + stride[0]*y));
> >>                  b = *((const uint16_t *)(src[1] + x*2 + stride[1]*y));
> >>                  r = *((const uint16_t *)(src[2] + x*2 + stride[2]*y));
> >>+                if (s->transparency)
> >>+                    a = *((const uint16_t *)(src[3] + x*2 + stride[2]*y));
> >                                                                ^^^^^^^^^
> >this looks wrong
> 
> Stupid typo not seen as strides are same here, fixed (I reviewed again the
> patch, looks like it is the only one in the list of changes).
> 
> Updated patch attached.
> 
> >
> >
> >also what speed effect do thes changes to the inner loop have ?
> 
> I see same performance, which I expect because when I do performance
> profiling I see that the transformation part is taking less than 0.1% of the
> CPU time and I guess that the CPU prediction stuff does its job here.

I dont belive that the transformation takes less than 0.1% of the time
please use START/STOP_TIMER for benchmarking code

> 

>  ffv1dec.c          |   14 ++++++++++----
>  ffv1dec_template.c |    9 +++++++--
>  ffv1enc.c          |   16 ++++++++++++++--
>  ffv1enc_template.c |   14 ++++++++++----
>  4 files changed, 41 insertions(+), 12 deletions(-)
> e625fd3a9e40ad1baee2e6bfe1d5cfb35b772f79  0001-avcodec-ffv1-Support-for-RGBA64-and-GBRAP16.patch
> From 0b9501984a8ce95d6e88a448683fd0d9d1473a08 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome at mediaarea.net>
> Date: Thu, 1 Feb 2018 13:11:53 +0100
> Subject: [PATCH] avcodec/ffv1: Support for RGBA64 and GBRAP16

will apply

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180206/bd8fe80a/attachment.sig>