diff mbox

[FFmpeg-devel] libavformat/nutenc.c: Use ffv1 as default video encoder for NUT muxing

Message ID 20170927231450.30967-1-leo.izen@gmail.com
State New
Headers show

Commit Message

Leo Izen Sept. 27, 2017, 11:14 p.m. UTC 
---
 libavformat/nutenc.c    |  2 +-
 tests/ref/lavf/nut      |  6 +++---
 tests/ref/seek/lavf-nut | 54 ++++++++++++++++++++++++-------------------------
 3 files changed, 31 insertions(+), 31 deletions(-)

Comments

Michael Niedermayer Sept. 28, 2017, 8:51 p.m. UTC | #1 
On Wed, Sep 27, 2017 at 07:14:50PM -0400, Leo Izen wrote:
> ---
>  libavformat/nutenc.c    |  2 +-
>  tests/ref/lavf/nut      |  6 +++---
>  tests/ref/seek/lavf-nut | 54 ++++++++++++++++++++++++-------------------------
>  3 files changed, 31 insertions(+), 31 deletions(-)
> 
> diff --git a/libavformat/nutenc.c b/libavformat/nutenc.c
> index a92ff55c01..6e96626ff0 100644
> --- a/libavformat/nutenc.c
> +++ b/libavformat/nutenc.c
> @@ -1221,7 +1221,7 @@ AVOutputFormat ff_nut_muxer = {
>      .priv_data_size = sizeof(NUTContext),
>      .audio_codec    = CONFIG_LIBVORBIS ? AV_CODEC_ID_VORBIS :
>                        CONFIG_LIBMP3LAME ? AV_CODEC_ID_MP3 : AV_CODEC_ID_MP2,
> -    .video_codec    = AV_CODEC_ID_MPEG4,
> +    .video_codec    = AV_CODEC_ID_FFV1,
>      .write_header   = nut_write_header,
>      .write_packet   = nut_write_packet,
>      .write_trailer  = nut_write_trailer,

Why?

also this breaks existing code and command lines which expect mpeg4 as
default

[...]
Leo Izen Oct. 2, 2017, 9:18 p.m. UTC | #2 
On 09/28/2017 04:51 PM, Michael Niedermayer wrote:
> On Wed, Sep 27, 2017 at 07:14:50PM -0400, Leo Izen wrote:
>> ---
>>   libavformat/nutenc.c    |  2 +-
>>   tests/ref/lavf/nut      |  6 +++---
>>   tests/ref/seek/lavf-nut | 54 ++++++++++++++++++++++++-------------------------
>>   3 files changed, 31 insertions(+), 31 deletions(-)
>>
>> diff --git a/libavformat/nutenc.c b/libavformat/nutenc.c
>> index a92ff55c01..6e96626ff0 100644
>> --- a/libavformat/nutenc.c
>> +++ b/libavformat/nutenc.c
>> @@ -1221,7 +1221,7 @@ AVOutputFormat ff_nut_muxer = {
>>       .priv_data_size = sizeof(NUTContext),
>>       .audio_codec    = CONFIG_LIBVORBIS ? AV_CODEC_ID_VORBIS :
>>                         CONFIG_LIBMP3LAME ? AV_CODEC_ID_MP3 : AV_CODEC_ID_MP2,
>> -    .video_codec    = AV_CODEC_ID_MPEG4,
>> +    .video_codec    = AV_CODEC_ID_FFV1,
>>       .write_header   = nut_write_header,
>>       .write_packet   = nut_write_packet,
>>       .write_trailer  = nut_write_trailer,
> Why?
>
> also this breaks existing code and command lines which expect mpeg4 as
> default

This was a patch I submitted as suggested by Mark Thompson on IRC, 
2017-09-27:

[18:47:51] <thebombzen> while we're on automatic order, for NUT, mpeg4 
gets picked by default rather than ffv1
[18:47:59] <thebombzen> this doesn't make sense
[18:49:22] <@jkqxz> thebombzen:  Yes.  Change 
<http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/nutenc.c;h=a92ff55c01373bd7db36088114888a1c1d15ef69;hb=HEAD#l1224>?
[18:49:48] <thebombzen> sure. I'll send a patch to the mailing list

A discussion on whether this change is a good one would require their input.

Thanks,
Leo Izen (thebombzen)
Carl Eugen Hoyos Oct. 2, 2017, 9:50 p.m. UTC | #3 
2017-10-02 23:18 GMT+02:00 Leo Izen <leo.izen@gmail.com>:
> On 09/28/2017 04:51 PM, Michael Niedermayer wrote:
>>
>> On Wed, Sep 27, 2017 at 07:14:50PM -0400, Leo Izen wrote:

>>> -    .video_codec    = AV_CODEC_ID_MPEG4,
>>> +    .video_codec    = AV_CODEC_ID_FFV1,
>>>       .write_header   = nut_write_header,
>>>       .write_packet   = nut_write_packet,
>>>       .write_trailer  = nut_write_trailer,
>>
>> Why?
>>
>> also this breaks existing code and command lines
>> which expect mpeg4 as default
>
> This was a patch I submitted as suggested by Mark
> Thompson on IRC, 2017-09-27:
>
> [18:47:51] <thebombzen> while we're on automatic order, for NUT,
> mpeg4 gets picked by default rather than ffv1

(The irc paste imo contradicts your claim above)

I don't think ffv1 is a useful default video encoder for nut.

Carl Eugen
Michael Niedermayer Oct. 3, 2017, 7:45 p.m. UTC | #4 
On Mon, Oct 02, 2017 at 11:50:45PM +0200, Carl Eugen Hoyos wrote:
> 2017-10-02 23:18 GMT+02:00 Leo Izen <leo.izen@gmail.com>:
> > On 09/28/2017 04:51 PM, Michael Niedermayer wrote:
> >>
> >> On Wed, Sep 27, 2017 at 07:14:50PM -0400, Leo Izen wrote:
> 
> >>> -    .video_codec    = AV_CODEC_ID_MPEG4,
> >>> +    .video_codec    = AV_CODEC_ID_FFV1,
> >>>       .write_header   = nut_write_header,
> >>>       .write_packet   = nut_write_packet,
> >>>       .write_trailer  = nut_write_trailer,
> >>
> >> Why?
> >>
> >> also this breaks existing code and command lines
> >> which expect mpeg4 as default
> >
> > This was a patch I submitted as suggested by Mark
> > Thompson on IRC, 2017-09-27:
> >
> > [18:47:51] <thebombzen> while we're on automatic order, for NUT,
> > mpeg4 gets picked by default rather than ffv1
> 
> (The irc paste imo contradicts your claim above)
> 
> I don't think ffv1 is a useful default video encoder for nut.

yes
ffv1 is a great choice as default for lossless encoding.
but if lossless is not the goal then a lossless codec is not a good
default.


[...]
diff mbox

Patch

diff --git a/libavformat/nutenc.c b/libavformat/nutenc.c
index a92ff55c01..6e96626ff0 100644
--- a/libavformat/nutenc.c
+++ b/libavformat/nutenc.c
@@ -1221,7 +1221,7 @@  AVOutputFormat ff_nut_muxer = {
     .priv_data_size = sizeof(NUTContext),
     .audio_codec    = CONFIG_LIBVORBIS ? AV_CODEC_ID_VORBIS :
                       CONFIG_LIBMP3LAME ? AV_CODEC_ID_MP3 : AV_CODEC_ID_MP2,
-    .video_codec    = AV_CODEC_ID_MPEG4,
+    .video_codec    = AV_CODEC_ID_FFV1,
     .write_header   = nut_write_header,
     .write_packet   = nut_write_packet,
     .write_trailer  = nut_write_trailer,
diff --git a/tests/ref/lavf/nut b/tests/ref/lavf/nut
index 1c3d7107da..ea2f89124d 100644
--- a/tests/ref/lavf/nut
+++ b/tests/ref/lavf/nut
@@ -1,3 +1,3 @@ 
-424e8037d7b6f3d3c09cf76bf06a63cb *./tests/data/lavf/lavf.nut
-319958 ./tests/data/lavf/lavf.nut
-./tests/data/lavf/lavf.nut CRC=0xec6c3c68
+cb603de6d9dce6eccef210f8b73980e5 *./tests/data/lavf/lavf.nut
+1353826 ./tests/data/lavf/lavf.nut
+./tests/data/lavf/lavf.nut CRC=0x233df7f5
diff --git a/tests/ref/seek/lavf-nut b/tests/ref/seek/lavf-nut
index b2da52629b..d2191a6efa 100644
--- a/tests/ref/seek/lavf-nut
+++ b/tests/ref/seek/lavf-nut
@@ -1,53 +1,53 @@ 
-ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    395 size:   208
+ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    364 size:   208
 ret: 0         st:-1 flags:0  ts:-1.000000
-ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    623 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    592 size: 55646
 ret: 0         st:-1 flags:1  ts: 1.894167
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.970918 pts: 0.970918 pos:1297517 size: 55895
 ret: 0         st: 0 flags:0  ts: 0.788340
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.970918 pts: 0.970918 pos:1297517 size: 55895
 ret: 0         st: 0 flags:1  ts:-0.317500
-ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    623 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    592 size: 55646
 ret: 0         st: 1 flags:0  ts: 2.576667
-ret: 0         st: 1 flags:1 dts: 0.862041 pts: 0.862041 pos: 271315 size:   209
+ret: 0         st: 1 flags:1 dts: 0.940408 pts: 0.940408 pos:1297077 size:   209
 ret: 0         st: 1 flags:1  ts: 1.470839
-ret: 0         st: 1 flags:1 dts: 0.862041 pts: 0.862041 pos: 271315 size:   209
+ret: 0         st: 1 flags:1 dts: 0.940408 pts: 0.940408 pos:1297077 size:   209
 ret: 0         st:-1 flags:0  ts: 0.365002
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 650696 size: 54610
 ret: 0         st:-1 flags:1  ts:-0.740831
-ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    623 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    592 size: 55646
 ret: 0         st: 0 flags:0  ts: 2.153340
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.970918 pts: 0.970918 pos:1297517 size: 55895
 ret: 0         st: 0 flags:1  ts: 1.047500
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.970918 pts: 0.970918 pos:1297517 size: 55895
 ret: 0         st: 1 flags:0  ts:-0.058322
-ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    395 size:   208
+ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    364 size:   208
 ret: 0         st: 1 flags:1  ts: 2.835828
-ret: 0         st: 1 flags:1 dts: 0.862041 pts: 0.862041 pos: 271315 size:   209
+ret: 0         st: 1 flags:1 dts: 0.940408 pts: 0.940408 pos:1297077 size:   209
 ret: 0         st:-1 flags:0  ts: 1.730004
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.970918 pts: 0.970918 pos:1297517 size: 55895
 ret: 0         st:-1 flags:1  ts: 0.624171
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 650696 size: 54610
 ret: 0         st: 0 flags:0  ts:-0.481660
-ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    623 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    592 size: 55646
 ret: 0         st: 0 flags:1  ts: 2.412500
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.970918 pts: 0.970918 pos:1297517 size: 55895
 ret: 0         st: 1 flags:0  ts: 1.306667
-ret: 0         st: 1 flags:1 dts: 0.862041 pts: 0.862041 pos: 271315 size:   209
+ret: 0         st: 1 flags:1 dts: 0.940408 pts: 0.940408 pos:1297077 size:   209
 ret: 0         st: 1 flags:1  ts: 0.200839
-ret: 0         st: 1 flags:1 dts: 0.182857 pts: 0.182857 pos:  71957 size:   209
+ret: 0         st: 1 flags:1 dts: 0.182857 pts: 0.182857 pos: 271744 size:   209
 ret: 0         st:-1 flags:0  ts:-0.904994
-ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    623 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    592 size: 55646
 ret: 0         st:-1 flags:1  ts: 1.989173
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.970918 pts: 0.970918 pos:1297517 size: 55895
 ret: 0         st: 0 flags:0  ts: 0.883340
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.970918 pts: 0.970918 pos:1297517 size: 55895
 ret: 0         st: 0 flags:1  ts:-0.222500
-ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    623 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    592 size: 55646
 ret: 0         st: 1 flags:0  ts: 2.671678
-ret: 0         st: 1 flags:1 dts: 0.862041 pts: 0.862041 pos: 271315 size:   209
+ret: 0         st: 1 flags:1 dts: 0.940408 pts: 0.940408 pos:1297077 size:   209
 ret: 0         st: 1 flags:1  ts: 1.565850
-ret: 0         st: 1 flags:1 dts: 0.862041 pts: 0.862041 pos: 271315 size:   209
+ret: 0         st: 1 flags:1 dts: 0.940408 pts: 0.940408 pos:1297077 size:   209
 ret: 0         st:-1 flags:0  ts: 0.460008
-ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 146490 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.490918 pts: 0.490918 pos: 650696 size: 54610
 ret: 0         st:-1 flags:1  ts:-0.645825
-ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    623 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.010918 pts: 0.010918 pos:    592 size: 55646