Re: [FFmpeg-devel] [FFmpeg-cvslog] lavf/cafenc: Allow muxing opus.

2017-10-17 Thread James Almer
On 10/17/2017 10:10 PM, Carl Eugen Hoyos wrote:
> 2017-10-17 23:48 GMT+02:00 James Almer :
>> On 10/17/2017 6:20 PM, Carl Eugen Hoyos wrote:
>>> 2017-10-17 22:20 GMT+02:00 James Almer :
> ffmpeg | branch: master | Carl Eugen Hoyos  > | Tue Oct 17 21:35:28 
> 2017 +0200| [a3accd0c3768933f15422c9dec596da0f436d786] | committer: Carl 
> Eugen Hoyos
>
> lavf/cafenc: Allow muxing opus.
>
> QuickTime does not require the (unknown) kuki chunk for decoding.

 This at the very least requires limiting muxing to mono and stereo
 streams (The only kind of streams that work without channel mapping
 information in extradata),
>>>
>>> Done, thank you!
>>>
 and a check for -strict experimental.
>>>
>>> Why?
>>
>> Because we're creating these files blindly
> 
> This is not correct (and offending, no matter the issue
> with the original patch).

I don't see how it can be offending. But in any case, would "without
understanding all the encapsulation details" instead of blindly be less
offensive?

> 
>> without a proper RE attempt
> 
> How do you know?

You specifically called the contents of the kuki chunk "unknown" in the
commit message. That pretty much implies there has been no concrete
attempt to RE it. Not to mention you'd have added support for it otherwise.

> 
>> or following any kind of encapsulation spec. And considering the
>> official encoder/muxer adds a codec specific chunk we don't, the more
>> reason to consider a muxing as barebones as this as something
>> potentially non-compliant.
> 
> Imo, this is not a helpful approach:
> The "official" demuxer/decoder accept the files, how does
> it make sense to argue they are potentially non-compliant?

It happening to work or be accepted isn't the same as being spec
compliant. Some specs can be very pedantic in the most silly of ways
with small details and declare something that pretty much any
implementation accepts as non-compliant. See things like MPEG/WebM DASH.

In this case, the spec says "If the audio data format contained in a CAF
file requires magic cookie data, the file must have this chunk", which
is apparently followed by the official muxer application, but not by
Quicktime since according to your tests it ignores its absence.

> 
 Also, I'd have rather waited for Apple to update their docs about Opus
 in CAF.
>>>
>>> As in: Until Godot arrives?
>>>
>>> Sorry, I don't think it makes much sense to wait for a
>>> specification that to best of our knowledge may never
>>> appear.
>>
>> Apple published the spec for CAF
> 
>> detailing encapsulation details for the codecs it supports
> 
> Depending on the definition of "detailing":
> Yes.
> (For most codecs, the encapsulation details had to be
> guessed.)
> 
>> including the contents of the kuki chunk for most of them.
> 
> Could you elaborate? This is not how I remember it.

The spec mentions that for AAC, the kuki chunk contains the same data as
the esds atom in ISOBMFF. And then there's
https://github.com/macosforge/alac/blob/master/ALACMagicCookieDescription.txt
for ALAC. So at least the two most important ones.

I want to believe the same will be done for Opus, if anything by Xiph
(like they did with ISOBMFF).

> 
> Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [FFmpeg-cvslog] lavf/cafenc: Allow muxing opus.

2017-10-17 Thread Carl Eugen Hoyos
2017-10-17 23:48 GMT+02:00 James Almer :
> On 10/17/2017 6:20 PM, Carl Eugen Hoyos wrote:
>> 2017-10-17 22:20 GMT+02:00 James Almer :
 ffmpeg | branch: master | Carl Eugen Hoyos >>> > | Tue Oct 17 21:35:28 
 2017 +0200| [a3accd0c3768933f15422c9dec596da0f436d786] | committer: Carl 
 Eugen Hoyos

 lavf/cafenc: Allow muxing opus.

 QuickTime does not require the (unknown) kuki chunk for decoding.
>>>
>>> This at the very least requires limiting muxing to mono and stereo
>>> streams (The only kind of streams that work without channel mapping
>>> information in extradata),
>>
>> Done, thank you!
>>
>>> and a check for -strict experimental.
>>
>> Why?
>
> Because we're creating these files blindly

This is not correct (and offending, no matter the issue
with the original patch).

> without a proper RE attempt

How do you know?

> or following any kind of encapsulation spec. And considering the
> official encoder/muxer adds a codec specific chunk we don't, the more
> reason to consider a muxing as barebones as this as something
> potentially non-compliant.

Imo, this is not a helpful approach:
The "official" demuxer/decoder accept the files, how does
it make sense to argue they are potentially non-compliant?

>>> Also, I'd have rather waited for Apple to update their docs about Opus
>>> in CAF.
>>
>> As in: Until Godot arrives?
>>
>> Sorry, I don't think it makes much sense to wait for a
>> specification that to best of our knowledge may never
>> appear.
>
> Apple published the spec for CAF

> detailing encapsulation details for the codecs it supports

Depending on the definition of "detailing":
Yes.
(For most codecs, the encapsulation details had to be
guessed.)

> including the contents of the kuki chunk for most of them.

Could you elaborate? This is not how I remember it.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [FFmpeg-cvslog] lavf/cafenc: Allow muxing opus.

2017-10-17 Thread James Almer
On 10/17/2017 6:20 PM, Carl Eugen Hoyos wrote:
> 2017-10-17 22:20 GMT+02:00 James Almer :
>>> ffmpeg | branch: master | Carl Eugen Hoyos >> > | Tue Oct 17 21:35:28 
>>> 2017 +0200| [a3accd0c3768933f15422c9dec596da0f436d786] | committer: Carl 
>>> Eugen Hoyos
>>>
>>> lavf/cafenc: Allow muxing opus.
>>>
>>> QuickTime does not require the (unknown) kuki chunk for decoding.
>>
>> This at the very least requires limiting muxing to mono and stereo
>> streams (The only kind of streams that work without channel mapping
>> information in extradata),
> 
> Done, thank you!
> 
>> and a check for -strict experimental.
> 
> Why?

Because we're creating these files blindly, without a proper RE attempt
or following any kind of encapsulation spec. And considering the
official encoder/muxer adds a codec specific chunk we don't, the more
reason to consider a muxing as barebones as this as something
potentially non-compliant.

> 
>> Also, I'd have rather waited for Apple to update their docs about Opus
>> in CAF.
> 
> As in: Until Godot arrives?
> 
> Sorry, I don't think it makes much sense to wait for a
> specification that to best of our knowledge may never
> appear.

Apple published the spec for CAF, detailing encapsulation details for
the codecs it supports, including the contents of the kuki chunk for
most of them. It's to be expected that eventually they'll update it for
Opus.
While I'm not asking you to revert this addition, I'd prefer if you keep
the muxing code under the experimental check until we're 100% sure it's
correct, or until enough time has passed without the spec being updated.
See how VP9 in Mov was under such a check until the encapsulation spec
was frozen and made official.

PS: Maintainers lists Peter Ross as the CAF maintainer, but that seems
to refer to the demuxer seeing you're the author of the muxer. So
assuming you're maintaining it, could you list yourself there?

> 
> Thank you, Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [FFmpeg-cvslog] lavf/cafenc: Allow muxing opus.

2017-10-17 Thread Carl Eugen Hoyos
2017-10-17 22:20 GMT+02:00 James Almer :
>> ffmpeg | branch: master | Carl Eugen Hoyos > > | Tue Oct 17 21:35:28 
>> 2017 +0200| [a3accd0c3768933f15422c9dec596da0f436d786] | committer: Carl 
>> Eugen Hoyos
>>
>> lavf/cafenc: Allow muxing opus.
>>
>> QuickTime does not require the (unknown) kuki chunk for decoding.
>
> This at the very least requires limiting muxing to mono and stereo
> streams (The only kind of streams that work without channel mapping
> information in extradata),

Done, thank you!

> and a check for -strict experimental.

Why?

> Also, I'd have rather waited for Apple to update their docs about Opus
> in CAF.

As in: Until Godot arrives?

Sorry, I don't think it makes much sense to wait for a
specification that to best of our knowledge may never
appear.

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [FFmpeg-cvslog] lavf/cafenc: Allow muxing opus.

2017-10-17 Thread James Almer
> ffmpeg | branch: master | Carl Eugen Hoyos  > | Tue Oct 17 21:35:28 
> 2017 +0200| [a3accd0c3768933f15422c9dec596da0f436d786] | committer: Carl 
> Eugen Hoyos
>
> lavf/cafenc: Allow muxing opus.
>
> QuickTime does not require the (unknown) kuki chunk for decoding.

This at the very least requires limiting muxing to mono and stereo
streams (The only kind of streams that work without channel mapping
information in extradata), and a check for -strict experimental.

Also, I'd have rather waited for Apple to update their docs about Opus
in CAF. Quicktime seemingly playing such files isn't reason enough to
risk creating potentially non-spec compliant files.

>
> >/http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=a3accd0c3768933f15422c9dec596da0f436d786
> /---
>
>  libavformat/cafenc.c  | 3 ++-
>  libavformat/version.h | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/cafenc.c b/libavformat/cafenc.c
> index f550cd965a..211b046563 100644
> --- a/libavformat/cafenc.c
> +++ b/libavformat/cafenc.c
> @@ -81,6 +81,8 @@ static uint32_t samples_per_packet(enum AVCodecID codec_id, 
> int channels, int bl
>  return 320;
>  case AV_CODEC_ID_MP1:
>  return 384;
> +case AV_CODEC_ID_OPUS:
> +return 960;
>  case AV_CODEC_ID_MP2:
>  case AV_CODEC_ID_MP3:
>  return 1152;
> @@ -117,7 +119,6 @@ static int caf_write_header(AVFormatContext *s)
>  
>  switch (par->codec_id) {
>  case AV_CODEC_ID_AAC:
> -case AV_CODEC_ID_OPUS:
>  av_log(s, AV_LOG_ERROR, "muxing codec currently unsupported\n");
>  return AVERROR_PATCHWELCOME;
>  }
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 828be14b20..22f82a37f7 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -33,7 +33,7 @@
>  // Also please add any ticket numbers that you believe might be affected here
>  #define LIBAVFORMAT_VERSION_MAJOR  57
>  #define LIBAVFORMAT_VERSION_MINOR  84
> -#define LIBAVFORMAT_VERSION_MICRO 100
> +#define LIBAVFORMAT_VERSION_MICRO 101
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> LIBAVFORMAT_VERSION_MINOR, \

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel