On 8/1/2017 7:48 PM, 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.
>
> Also add simple FATE tests, based on output produced by the NDI SDK.
>
> Add myself as speedhq maintainer, per request.
This should ideally be split in two or three patches, one per
addition/change.
> ---
> 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)
This depends also on the rawvideo demuxer, so it needs call DEMDEC.
Also, use FATE_SAMPLES_FFMPEG, since it's a test added in our tree and
not elsewhere.
FATE_SAMPLES_FFMPEG-$(call DEMDEC, RAWVIDEO, SPEEDHQ) += $(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
Use -c:v and -tag:v, not -codec and -vtag.
> 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
This is wrong. You're trying to add a test meant for encoders.
> +
> 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
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
