Re: [FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding
On Wed, Aug 02, 2017 at 03:18:49AM +0200, Michael Niedermayer wrote: > This seems to break a full "make fate" OK, removing the offending line and resending. /* Steinar */ -- Homepage: https://www.sesse.net/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding
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.cMichael 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" Gu9456 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 00..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
>
Re: [FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding
On Wed, Aug 02, 2017 at 12:51:17AM +0200, Steinar H. Gunderson wrote: > On Wed, Aug 02, 2017 at 12:48:38AM +0200, Steinar H. Gunderson wrote: > > Also add simple FATE tests, based on output produced by the NDI SDK. > > Here are the required samples. I couldn't find much documentation on how to > add tests to FATE, so everything has been done by cargo culting. files uploaded thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding
On Wed, Aug 02, 2017 at 12:48:38AM +0200, 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. This seems to break a full "make fate" reference file './tests/ref/vsynth/vsynth1-speedhq' not found Test vsynth1-speedhq failed. Look at tests/data/fate/vsynth1-speedhq.err for details. adding ./tests/ref/vsynth/vsynth1-speedhq results the in: make: *** [fate-vsynth1-speedhq] Error 1 the error is Unknown encoder 'speedhq' [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding
On Wed, Aug 02, 2017 at 12:48:38AM +0200, Steinar H. Gunderson wrote: > Also add simple FATE tests, based on output produced by the NDI SDK. Here are the required samples. I couldn't find much documentation on how to add tests to FATE, so everything has been done by cargo culting. /* Steinar */ -- Homepage: https://www.sesse.net/ fate-speedhq-samples.tar.gz Description: application/gzip ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding
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.cMichael 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" Gu9456 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 00..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 \
Re: [FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding
On Fri, Jul 28, 2017 at 11:57:24AM +0200, 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 | 15 +-- > 1 file changed, 9 insertions(+), 6 deletions(-) can you add a fate test for this ? you also may want to add yourself to MAINTAINERs for speedhq.c it seems you are the one primarly maintaining this code thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The bravest are surely those who have the clearest vision of what is before them, glory and danger alike, and yet notwithstanding go out to meet it. -- Thucydides signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] speedhq: fix behavior of single-field decoding
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 | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
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 {
--
2.13.3
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
