diff mbox

[FFmpeg-devel,3/4] avformat/mpegenc - accept PCM_DVD streams

Message ID fb8a818e-bc83-0a90-0153-07cf528e3e4b@gmail.com
State Superseded
Headers show

Commit Message

Gyan Feb. 12, 2018, 7:06 p.m. UTC
On 2/3/2018 3:59 AM, Michael Niedermayer wrote:

>> Subject: [PATCH v2] avformat/mpegenc - accept PCM_DVD streams
>>
>> PCM_S16BE stream packets in MPEG-PS have a 3-byte header
>> and recognized as PCM_DVD by the demuxer which prevents
>> their proper remuxing in MPEG-1/2 PS.
> 
> its probably a good idea to add a fate test for this too.
> (could be in a seperate patch)
> 

FATE patch and sample file attached. Sample file should go into (new) 
mpegps directory in suite.


Regards,
Gyan
From cbeb8915b4149abfc33936a94c3280cf6872d9e6 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <gyandoshi@gmail.com>
Date: Mon, 12 Feb 2018 23:59:09 +0530
Subject: [PATCH] fate/mpegps: add fate test for remux of 16-bit PCM_DVD stream
 in MPEG-PS

---
 tests/Makefile        | 1 +
 tests/fate/mpegps.mak | 9 +++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 tests/fate/mpegps.mak

Comments

Michael Niedermayer Feb. 13, 2018, 10:20 p.m. UTC | #1
On Tue, Feb 13, 2018 at 12:36:15AM +0530, Gyan Doshi wrote:
> 
> On 2/3/2018 3:59 AM, Michael Niedermayer wrote:
> 
> >>Subject: [PATCH v2] avformat/mpegenc - accept PCM_DVD streams
> >>
> >>PCM_S16BE stream packets in MPEG-PS have a 3-byte header
> >>and recognized as PCM_DVD by the demuxer which prevents
> >>their proper remuxing in MPEG-1/2 PS.
> >
> >its probably a good idea to add a fate test for this too.
> >(could be in a seperate patch)
> >
> 
> FATE patch and sample file attached. Sample file should go into (new) mpegps
> directory in suite.
> 
> 
> Regards,
> Gyan

>  Makefile        |    1 +
>  fate/mpegps.mak |    9 +++++++++
>  2 files changed, 10 insertions(+)
> 3141a186401509be875d81a64bcaf1bb598e1f65  0001-fate-mpegps-add-fate-test-for-remux-of-16-bit-PCM_DV.patch
> From cbeb8915b4149abfc33936a94c3280cf6872d9e6 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <gyandoshi@gmail.com>
> Date: Mon, 12 Feb 2018 23:59:09 +0530
> Subject: [PATCH] fate/mpegps: add fate test for remux of 16-bit PCM_DVD stream
>  in MPEG-PS
> 
> ---
>  tests/Makefile        | 1 +
>  tests/fate/mpegps.mak | 9 +++++++++
>  2 files changed, 10 insertions(+)
>  create mode 100644 tests/fate/mpegps.mak
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 327e3f4420..f1ac610454 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -149,6 +149,7 @@ include $(SRC_PATH)/tests/fate/mov.mak
>  include $(SRC_PATH)/tests/fate/mp3.mak
>  include $(SRC_PATH)/tests/fate/mpc.mak
>  include $(SRC_PATH)/tests/fate/mpeg4.mak
> +include $(SRC_PATH)/tests/fate/mpegps.mak
>  include $(SRC_PATH)/tests/fate/mpegts.mak
>  include $(SRC_PATH)/tests/fate/mxf.mak
>  include $(SRC_PATH)/tests/fate/opus.mak
> diff --git a/tests/fate/mpegps.mak b/tests/fate/mpegps.mak
> new file mode 100644
> index 0000000000..56a88640bc
> --- /dev/null
> +++ b/tests/fate/mpegps.mak
> @@ -0,0 +1,9 @@
> +# This tests that the mpegps muxer supports a 16-bit pcm_dvd stream in remuxing (-c:a copy)
> +FATE_MPEGPS-$(call DEMMUX, MPEGPS, MPEG1SYSTEM) += fate-mpegps-pcm_dvd-remux
> +fate-mpegps-pcm_dvd-remux: CMD = md5 -i $(TARGET_SAMPLES)/mpegps/pcm_aud.mpg -vn -c copy -fflags +bitexact -f mpeg
> +fate-mpegps-pcm_dvd-remux: CMP = oneline
> +fate-mpegps-pcm_dvd-remux: REF = 28e5de42b1b00d7fa6f98df6a82d122c

testing with framecrc in addition to this might be usefull too
these single line md5s tend to be harder to interpret if they change

[...]
Michael Niedermayer Feb. 15, 2018, 3:28 a.m. UTC | #2
On Tue, Feb 13, 2018 at 12:36:15AM +0530, Gyan Doshi wrote:
> 
> On 2/3/2018 3:59 AM, Michael Niedermayer wrote:
> 
> >>Subject: [PATCH v2] avformat/mpegenc - accept PCM_DVD streams
> >>
> >>PCM_S16BE stream packets in MPEG-PS have a 3-byte header
> >>and recognized as PCM_DVD by the demuxer which prevents
> >>their proper remuxing in MPEG-1/2 PS.
> >
> >its probably a good idea to add a fate test for this too.
> >(could be in a seperate patch)
> >
> 
> FATE patch and sample file attached. Sample file should go into (new) mpegps
> directory in suite.

have you checked that none of the existing fate samples can be used ?

[...]
diff mbox

Patch

diff --git a/tests/Makefile b/tests/Makefile
index 327e3f4420..f1ac610454 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -149,6 +149,7 @@  include $(SRC_PATH)/tests/fate/mov.mak
 include $(SRC_PATH)/tests/fate/mp3.mak
 include $(SRC_PATH)/tests/fate/mpc.mak
 include $(SRC_PATH)/tests/fate/mpeg4.mak
+include $(SRC_PATH)/tests/fate/mpegps.mak
 include $(SRC_PATH)/tests/fate/mpegts.mak
 include $(SRC_PATH)/tests/fate/mxf.mak
 include $(SRC_PATH)/tests/fate/opus.mak
diff --git a/tests/fate/mpegps.mak b/tests/fate/mpegps.mak
new file mode 100644
index 0000000000..56a88640bc
--- /dev/null
+++ b/tests/fate/mpegps.mak
@@ -0,0 +1,9 @@ 
+# This tests that the mpegps muxer supports a 16-bit pcm_dvd stream in remuxing (-c:a copy)
+FATE_MPEGPS-$(call DEMMUX, MPEGPS, MPEG1SYSTEM) += fate-mpegps-pcm_dvd-remux
+fate-mpegps-pcm_dvd-remux: CMD = md5 -i $(TARGET_SAMPLES)/mpegps/pcm_aud.mpg -vn -c copy -fflags +bitexact -f mpeg
+fate-mpegps-pcm_dvd-remux: CMP = oneline
+fate-mpegps-pcm_dvd-remux: REF = 28e5de42b1b00d7fa6f98df6a82d122c
+
+
+FATE_SAMPLES_FFMPEG += $(FATE_MPEGPS-yes)
+fate-mpegps: $(FATE_MPEGPS-yes)