diff mbox

[FFmpeg-devel,v5,1/3] speedhq: fix behavior of single-field decoding

Message ID E1ddAce-000845-Sh@pannekake.samfundet.no
State Accepted
Commit effd2e7291a12760891289540f944d71ff32a1c8
Headers show

Commit Message

Steinar H. Gunderson Aug. 3, 2017, 7:22 a.m. UTC
The height convention for decoding frames with only a single field made sense
for compatibility with legacy decoders, but doesn't really match the convention
used by NDI, which is the primary (only?) user. Thus, change it to simply
assuming that if the two fields overlap, the frame is meant to be a single
field and the frame height matches the field height.
---
 libavcodec/speedhq.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

James Almer Aug. 3, 2017, 7:53 p.m. UTC | #1
On 8/3/2017 4:22 AM, Steinar H. Gunderson wrote:
> The height convention for decoding frames with only a single field made sense
> for compatibility with legacy decoders, but doesn't really match the convention
> used by NDI, which is the primary (only?) user. Thus, change it to simply
> assuming that if the two fields overlap, the frame is meant to be a single
> field and the frame height matches the field height.
> ---
>  libavcodec/speedhq.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/speedhq.c b/libavcodec/speedhq.c
> index 60efb0222b..47b1e4dc7a 100644
> --- a/libavcodec/speedhq.c
> +++ b/libavcodec/speedhq.c
> @@ -450,10 +450,13 @@ static int speedhq_decode_frame(AVCodecContext *avctx,
>      if (second_field_offset == 4) {
>          /*
>           * Overlapping first and second fields is used to signal
> -         * encoding only a single field (the second field then comes
> -         * as a separate, later frame).
> +         * encoding only a single field. In this case, "height"
> +         * is ambiguous; it could mean either the height of the
> +         * frame as a whole, or of the field. The former would make
> +         * more sense for compatibility with legacy decoders,
> +         * but this matches the convention used in NDI, which is
> +         * the primary user of this trick.
>           */
> -        frame->height >>= 1;
>          if ((ret = decode_speedhq_field(s, buf, buf_size, frame, 0, 4, buf_size, 1)) < 0)
>              return ret;
>      } else {
> 

Applied.
diff mbox

Patch

diff --git a/libavcodec/speedhq.c b/libavcodec/speedhq.c
index 60efb0222b..47b1e4dc7a 100644
--- a/libavcodec/speedhq.c
+++ b/libavcodec/speedhq.c
@@ -450,10 +450,13 @@  static int speedhq_decode_frame(AVCodecContext *avctx,
     if (second_field_offset == 4) {
         /*
          * Overlapping first and second fields is used to signal
-         * encoding only a single field (the second field then comes
-         * as a separate, later frame).
+         * encoding only a single field. In this case, "height"
+         * is ambiguous; it could mean either the height of the
+         * frame as a whole, or of the field. The former would make
+         * more sense for compatibility with legacy decoders,
+         * but this matches the convention used in NDI, which is
+         * the primary user of this trick.
          */
-        frame->height >>= 1;
         if ((ret = decode_speedhq_field(s, buf, buf_size, frame, 0, 4, buf_size, 1)) < 0)
             return ret;
     } else {