[FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding

Steinar H. Gunderson steinar+ffmpeg at gunderson.no
Wed Aug 2 01:48:38 EEST 2017


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.

Also add simple FATE tests, based on output produced by the NDI SDK.

Add myself as speedhq maintainer, per request.
---
 MAINTAINERS                            |  2 ++
 libavcodec/speedhq.c                   | 15 +++++++++------
 tests/Makefile                         |  1 +
 tests/fate/speedhq.mak                 |  8 ++++++++
 tests/fate/vcodec.mak                  |  2 ++
 tests/ref/fate/speedhq-422             |  6 ++++++
 tests/ref/fate/speedhq-422-singlefield |  6 ++++++
 7 files changed, 34 insertions(+), 6 deletions(-)
 create mode 100644 tests/fate/speedhq.mak
 create mode 100644 tests/ref/fate/speedhq-422
 create mode 100644 tests/ref/fate/speedhq-422-singlefield

diff --git a/MAINTAINERS b/MAINTAINERS
index ae0e08d121..ce5e1dae08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -233,6 +233,7 @@ Codecs:
   smvjpegdec.c                          Ash Hughes
   snow*                                 Michael Niedermayer, Loren Merritt
   sonic.c                               Alex Beregszaszi
+  speedhq.c                             Steinar H. Gunderson
   srt*                                  Aurelien Jacobs
   sunrast.c                             Ivo van Poorten
   svq3.c                                Michael Niedermayer
@@ -598,6 +599,7 @@ Reynaldo H. Verdejo Pinochet  6E27 CD34 170C C78E 4D4F 5F40 C18E 077F 3114 452A
 Robert Swain                  EE7A 56EA 4A81 A7B5 2001 A521 67FA 362D A2FC 3E71
 Sascha Sommer                 38A0 F88B 868E 9D3A 97D4 D6A0 E823 706F 1E07 0D3C
 Stefano Sabatini              0D0B AD6B 5330 BBAD D3D6 6A0C 719C 2839 FC43 2D5F
+Steinar H. Gunderson          C2E9 004F F028 C18E 4EAD DB83 7F61 7561 7797 8F76
 Stephan Hilb                  4F38 0B3A 5F39 B99B F505 E562 8D5C 5554 4E17 8863
 Tiancheng "Timothy" Gu        9456 AFC0 814A 8139 E994 8351 7FE6 B095 B582 B0D4
 Tim Nicholson                 38CF DB09 3ED0 F607 8B67 6CED 0C0B FC44 8B0B FC83
diff --git a/libavcodec/speedhq.c b/libavcodec/speedhq.c
index 60efb0222b..eca45beb67 100644
--- a/libavcodec/speedhq.c
+++ b/libavcodec/speedhq.c
@@ -448,12 +448,15 @@ static int speedhq_decode_frame(AVCodecContext *avctx,
     frame->key_frame = 1;
 
     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).
-         */
-        frame->height >>= 1;
+	/*
+	 * Overlapping first and second fields is used to signal
+	 * 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.
+	 */
         if ((ret = decode_speedhq_field(s, buf, buf_size, frame, 0, 4, buf_size, 1)) < 0)
             return ret;
     } else {
diff --git a/tests/Makefile b/tests/Makefile
index ab83ae855d..f9b9cf4188 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -164,6 +164,7 @@ include $(SRC_PATH)/tests/fate/qt.mak
 include $(SRC_PATH)/tests/fate/qtrle.mak
 include $(SRC_PATH)/tests/fate/real.mak
 include $(SRC_PATH)/tests/fate/screen.mak
+include $(SRC_PATH)/tests/fate/speedhq.mak
 include $(SRC_PATH)/tests/fate/source.mak
 include $(SRC_PATH)/tests/fate/subtitles.mak
 include $(SRC_PATH)/tests/fate/utvideo.mak
diff --git a/tests/fate/speedhq.mak b/tests/fate/speedhq.mak
new file mode 100644
index 0000000000..a5c2fb99a9
--- /dev/null
+++ b/tests/fate/speedhq.mak
@@ -0,0 +1,8 @@
+FATE_SPEEDHQ = fate-speedhq-422                                           \
+               fate-speedhq-422-singlefield                               \
+
+FATE_SAMPLES_AVCONV-$(call ALLYES, SPEEDHQ_DECODER) += $(FATE_SPEEDHQ)
+fate-speedhq: $(FATE_SPEEDHQ)
+
+fate-speedhq-422:             CMD = framecrc -flags +bitexact -f rawvideo -codec speedhq -vtag SHQ2 -video_size 112x64 -i $(TARGET_SAMPLES)/speedhq/progressive.shq2 -pix_fmt yuv422p
+fate-speedhq-422-singlefield: CMD = framecrc -flags +bitexact -f rawvideo -codec speedhq -vtag SHQ2 -video_size 112x32 -i $(TARGET_SAMPLES)/speedhq/singlefield.shq2 -pix_fmt yuv422p
diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak
index 8c24510937..0a284ecbb9 100644
--- a/tests/fate/vcodec.mak
+++ b/tests/fate/vcodec.mak
@@ -386,6 +386,8 @@ fate-vsynth%-snow-hpel:          ENCOPTS = -qscale 2              \
 fate-vsynth%-snow-ll:            ENCOPTS = -qscale .001 -pred 1 \
                                            -flags +mv4+qpel
 
+FATE_VCODEC-$(call ALLYES, SPEEDHQ_DECODER) += speedhq
+
 FATE_VCODEC-$(call ENCDEC, SVQ1, MOV)   += svq1
 fate-vsynth%-svq1:               ENCOPTS = -qscale 3 -pix_fmt yuv410p
 fate-vsynth%-svq1:               FMT     = mov
diff --git a/tests/ref/fate/speedhq-422 b/tests/ref/fate/speedhq-422
new file mode 100644
index 0000000000..7bb0d2388d
--- /dev/null
+++ b/tests/ref/fate/speedhq-422
@@ -0,0 +1,6 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 112x64
+#sar 0: 0/1
+0,          0,          0,        1,    14336, 0x9bb6dc6d
diff --git a/tests/ref/fate/speedhq-422-singlefield b/tests/ref/fate/speedhq-422-singlefield
new file mode 100644
index 0000000000..343c52645c
--- /dev/null
+++ b/tests/ref/fate/speedhq-422-singlefield
@@ -0,0 +1,6 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 112x32
+#sar 0: 0/1
+0,          0,          0,        1,     7168, 0x75de4109
-- 
2.13.3



More information about the ffmpeg-devel mailing list