[FFmpeg-devel] lavc/cfhd: added interlaced frame decoding

Submitted by Gagandeep Singh on March 19, 2018, 6:45 a.m.

Details

Message ID CAOi=zRtVSymT8Egt=G3MSuN+QxEGORPpMpEJ2d+wKAqFF9g6-A@mail.gmail.com
State New
Headers show

Commit Message

Gagandeep Singh March 19, 2018, 6:45 a.m.
On Sat, Mar 17, 2018 at 5:00 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
wrote:

> 2018-03-17 10:42 GMT+01:00, Gagandeep Singh <deepgagan231197@gmail.com>:
> > ticket #5522: interlaced frame required horizontal-temporal inverse
> > transform. though the output is not satisfactory yet.
>
> > diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
> > index a064cd1599..b17c7c1dc5 100644
> > --- a/libavcodec/cfhd.c
> > +++ b/libavcodec/cfhd.c
> > @@ -50,8 +50,11 @@ enum CFHDParam {
> >      ChannelWidth     = 104,
> >      ChannelHeight    = 105,
> >      PrescaleShift    = 109,
> > +    Progressive      =  68,
>
> I suspect this can be ordered better.
>
> >  };
> >
> > +
> > +
>
> Please do not add random cosmetic changes to your patch.
>
> [...]
>
> > +static inline void interlaced_vertical_filter(int16_t *output, int16_t
> > *low, int16_t *high,
> > +                         int width, int linesize, int plane)
> > +{
> > +    int i;
>
> > +    int16_t even, odd;
>
> Could the code be simplified by using an unsigned type?
> Why not a standard type?
>
> > +    for (i = 0; i < width; i++) {
>
> > +
> > +
>
> Maybe you disagree, but these empty lines make
> reading the code more difficult imo.
>
> > +      even = (*low - *high)/2;
> > +      odd  = (*low + *high)/2;
> > +
> > +      if (even > 1023) even = 1023;
> > +      if (even < 0) even = 0;
> > +      if (odd > 1023) odd = 1023;
> > +      if (odd < 0) odd = 0;
>
> FFMIN / FFMAX
>
> [...]
>
> > +      }
> > +      else {
>
> Please merge these lines.
>
> Thank you, Carl Eugen


>


 libavcodec/cfhd.c | 107
+++++++++++++++++++++++++++++-------------------------
 1 file changed, 58 insertions(+), 49 deletions(-)

       output[i + linesize] = odd;
@@ -163,6 +159,23 @@ static inline void interlaced_vertical_filter(int16_t
*output, int16_t *low, int
       high++;
     }
 }
+
+static inline void horiz_haar_filter(int16_t *output, int16_t *low,
int16_t *high,
+                         int width)
+{
+    int i;
+    int even, odd;
+    for (i = 0; i < width; i+=2) {
+      even = (*low - *high);
+      odd  = (*low + *high);
+
+      output[i] = even;
+      output[i + 1] =odd;
+      low++;
+      high++;
+    }
+}
+
 static void horiz_filter(int16_t *output, int16_t *low, int16_t *high,
                          int width)
 {
@@ -222,7 +235,8 @@ static int alloc_buffers(AVCodecContext *avctx)
         int width  = i ? avctx->width  >> chroma_x_shift : avctx->width;
         int height = i ? avctx->height >> chroma_y_shift : avctx->height;
         ptrdiff_t stride           = FFALIGN(width  / 8, 8) * 8;
-        if (chroma_y_shift) height = FFALIGN(height / 8, 2) * 8;
+        if (chroma_y_shift)
+            height = FFALIGN(height / 8, 2) * 8;
         s->plane[i].width  = width;
         s->plane[i].height = height;
         s->plane[i].stride = stride;
@@ -233,7 +247,6 @@ static int alloc_buffers(AVCodecContext *avctx)
         h4 = h8 * 2;
         w2 = w4 * 2;
         h2 = h4 * 2;
-
         s->plane[i].idwt_buf =
             av_mallocz_array(height * stride,
sizeof(*s->plane[i].idwt_buf));
         s->plane[i].idwt_tmp =
@@ -273,7 +286,6 @@ static int alloc_buffers(AVCodecContext *avctx)
     s->a_height = s->coded_height;
     s->a_width  = s->coded_width;
     s->a_format = s->coded_format;
-
     return 0;
 }

@@ -304,8 +316,8 @@ static int cfhd_decode(AVCodecContext *avctx, void
*data, int *got_frame,
         if (abs_tag8 >= 0x60 && abs_tag8 <= 0x6f) {
             av_log(avctx, AV_LOG_DEBUG, "large len %x\n", ((tagu & 0xff)
<< 16) | data);
         } else if (tag == Progressive) {
-            av_log(avctx, AV_LOG_DEBUG, "Progressive?%"PRIu16"\n", data);
-            s->progressive = data;
+            av_log(avctx, AV_LOG_DEBUG, "Progressive?%"PRIu16"\n", 0x0001
& data);
+            s->progressive = 0x0001 & data;
         } else if (tag == ImageWidth) {
             av_log(avctx, AV_LOG_DEBUG, "Width %"PRIu16"\n", data);
             s->coded_width = data;
@@ -786,7 +798,6 @@ static int cfhd_decode(AVCodecContext *avctx, void
*data, int *got_frame,
         lowpass_height  = s->plane[plane].band[2][1].height;
         lowpass_width   = s->plane[plane].band[2][1].width;
         highpass_stride = s->plane[plane].band[2][1].stride;
-
         if (lowpass_height > s->plane[plane].band[2][1].a_height ||
lowpass_width > s->plane[plane].band[2][1].a_width ||
             !highpass_stride || s->plane[plane].band[2][1].width >
s->plane[plane].band[2][1].a_width) {
             av_log(avctx, AV_LOG_ERROR, "Invalid plane dimensions\n");
@@ -825,41 +836,39 @@ static int cfhd_decode(AVCodecContext *avctx, void
*data, int *got_frame,
               high += lowpass_width;
               dst  += pic->linesize[act_plane] / 2;
           }
-      }
-      else {
-
-        av_log(avctx, AV_LOG_DEBUG, "interlaced frame ? %d",
pic->interlaced_frame);
-        pic->interlaced_frame = 1;
-        low    = s->plane[plane].subband[0];
-        high   = s->plane[plane].subband[7];
-        output = s->plane[plane].l_h[6];
-        for (i = 0; i < lowpass_height; i++) {
-            horiz_filter(output, low, high, lowpass_width);
-            low    += lowpass_width;
-            high   += lowpass_width;
-            output += lowpass_width * 2;
-        }
+        } else {
+            av_log(avctx, AV_LOG_DEBUG, "interlaced frame ? %d",
pic->interlaced_frame);
+            pic->interlaced_frame = 1;
+            low    = s->plane[plane].subband[0];
+            high   = s->plane[plane].subband[7];
+            output = s->plane[plane].l_h[6];
+            for (i = 0; i < lowpass_height; i++) {
+                horiz_filter(output, low, high, lowpass_width);
+                low    += lowpass_width;
+                high   += lowpass_width;
+                output += lowpass_width * 2;
+            }

-        low    = s->plane[plane].subband[8];
-        high   = s->plane[plane].subband[9];
-        output = s->plane[plane].l_h[7];
-        for (i = 0; i < lowpass_height; i++) {
-            horiz_filter(output, low, high, lowpass_width);
-            low    += lowpass_width;
-            high   += lowpass_width;
-            output += lowpass_width * 2;
-        }
+            low    = s->plane[plane].subband[8];
+            high   = s->plane[plane].subband[9];
+            output = s->plane[plane].l_h[7];
+            for (i = 0; i < lowpass_height; i++) {
+                horiz_filter(output, low, high, lowpass_width);
+                low    += lowpass_width;
+                high   += lowpass_width;
+                output += lowpass_width * 2;
+            }

-        dst  = (int16_t *)pic->data[act_plane];
-        low  = s->plane[plane].l_h[6];
-        high = s->plane[plane].l_h[7];
-       for (i = 0; i < lowpass_height; i++) {
-            interlaced_vertical_filter(dst, low, high, lowpass_width * 2,
pic->linesize[act_plane]/2, act_plane);
-            low  += lowpass_width * 2;
-            high += lowpass_width * 2;
-            dst  += pic->linesize[act_plane];
-        }
-      }
+            dst  = (int16_t *)pic->data[act_plane];
+            low  = s->plane[plane].l_h[6];
+            high = s->plane[plane].l_h[7];
+            for (i = 0; i < lowpass_height; i++) {
+                interlaced_vertical_filter(dst, low, high, lowpass_width *
2,  pic->linesize[act_plane]/2, act_plane);
+                low  += lowpass_width * 2;
+                high += lowpass_width * 2;
+                dst  += pic->linesize[act_plane];
+            }
+          }
     }

Comments

Paul B Mahol March 19, 2018, 8:13 a.m.
On 3/19/18, Gagandeep Singh <deepgagan231197@gmail.com> wrote:
> On Sat, Mar 17, 2018 at 5:00 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
> wrote:
>

Wrong way.

Post patch against master ffmpeg, not against previous patch.

Patch hide | download patch | download mbox

diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
index b17c7c1dc5..57e0042d01 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -46,15 +46,13 @@  enum CFHDParam {
     SubbandNumber    =  48,
     Quantization     =  53,
     ChannelNumber    =  62,
+    Progressive      =  68,
     BitsPerComponent = 101,
     ChannelWidth     = 104,
     ChannelHeight    = 105,
     PrescaleShift    = 109,
-    Progressive      =  68,
 };

-
-
 static av_cold int cfhd_init(AVCodecContext *avctx)
 {
     CFHDContext *s = avctx->priv_data;
@@ -145,17 +143,15 @@  static inline void interlaced_vertical_filter(int16_t
*output, int16_t *low, int
                          int width, int linesize, int plane)
 {
     int i;
-    int16_t even, odd;
+    int even, odd;
     for (i = 0; i < width; i++) {
-
-
       even = (*low - *high)/2;
       odd  = (*low + *high)/2;

-      if (even > 1023) even = 1023;
-      if (even < 0) even = 0;
-      if (odd > 1023) odd = 1023;
-      if (odd < 0) odd = 0;
+      even = FFMIN(even, 1023);
+      even = FFMAX(even, 0);
+      odd  = FFMIN(odd, 1023);
+      odd  = FFMAX(odd, 0);

       output[i] = even;