On 7 January 2018 at 22:54, Aurelien Jacobs <au...@gnuage.org> wrote:
> On Sun, Jan 07, 2018 at 05:23:24PM +0000, Rostislav Pehlivanov wrote: > > On 6 January 2018 at 16:48, Aurelien Jacobs <au...@gnuage.org> wrote: > > > > > --- > > > Changelog | 2 +- > > > configure | 2 + > > > libavcodec/Makefile | 2 + > > > libavcodec/allcodecs.c | 1 + > > > libavcodec/aptx.c | 352 ++++++++++++++++++++++++++++++ > > > ++++++++++++++---- > > > libavcodec/avcodec.h | 1 + > > > libavcodec/codec_desc.c | 7 + > > > 7 files changed, 339 insertions(+), 28 deletions(-) > > > > > > No, don't add a new codec ID for what is very obviously a profile. > > > > Here's what you need to do: > > > > 1.) Add FF_PROFILE_APTX_HD to libavcodec/avcodec.h > > 2.) During parsing set par->profile to FF_PROFILE_APTX_HD > > Parsing ? Do you mean demuxer ? > There is no aptx parser for now. > Sorry, I meant demuxer. > > 3.) During decoding init set s->hd to avctx->profile == > FF_PROFILE_APTX_HD > > - or better yet don't add a bool variable but do this check every > time > > something is different > > The bool variable makes things easier because the avctx is not > accessible to functions using it. > That's okay, I don't mind. > Anyway, I do understand how I could use a profile instead of a new codec > ID, but I really don't understand what advantage it would bring ? > > For a codec known with one name, but supporting a lot of different > profiles with flags in the bitstream so that the decoder can adapt > itself to any profile, that makes a lot of sense. But for aptX and > aptX HD it doesn't make any sense to me. > It makes sense - HD is just a flavor of aptx. You can't call this a brand new codec - it just changes some tables and the way codewords are written (1 function). The only people who would call that brand new are in marketing and they're wrong. This is just changes the tables and the way codewords are written in order to optimize the codec for a different bitrate. That's all, not a new codec, its too small a change. Its a profile. > I don't think it would make the code simpler. > The decoder itself has no flag in the bitstream to adapt to the correct > "profile". > And I think it would be confusing for end user. aptX HD is publicly know > as a different codec than aptX. A user looking at the list of supported > codec would probably conclude that aptX is supported but not aptX HD. > > Also, the closest thing I can think as a standard container for aptX > is the bluetooth A2DP protocol. And in this protocol, aptX and aptX HD > are differentiated with a different codec ID, just the same way they > are differentiated from SBC or LDAC. > > So in the end, using different codec ID seems pretty natural, while > using different profiles seems akward and doesn't seem to bring any > advantage. > > Can you give any technical reason why think using profile would be > better ? > Yes, a big one - we don't bloat the already humongous API. You might say: "Well, we have hundreds of codec IDs, what's one more?". If we had a new codec ID for every flavor of every codec, we'd have thousands. The profile system saves us a decent dozen for when there are differences besides the "there's one extra block of bits/there's some reordering" - which remain hidden by the decoder. In that case, as far as the user is concerned they're not decoding a Chinese Y528 codec, an H264 variant with 1 extra bit of padding found in one province's TV broadcast to region lock, they're decoding regular H264. The profile systems allows us to discern between actually marketed variants of one codec with the same name, for example AAC-LC with AAC-LTP. The LTP profile of AAC is quite similar in what it aims to do compared to aptx HD - its just the same baseline codec with some changes to make it work better for a particular bitrate. > > Could you do this for sbc as well so we can get that merged finally? > > I already replied to this in the SBC thread, and the discussion should > probably continue there, but in the case of SBC, using profile is > even more problematic as SBC and mSBC don't support the same samplerate, > and having a single codec prevent to have too different samplerate > lists and prevent ffmpeg to automatically convert input audio to the > only samplerate supported by mSBC. > That's okay - for encoding switch the profile depending on both the avctx->profile setting and the samplerate and list all supported samplerates for all profiles in the AVCodec structure. We do something similar in the AAC encoder where we pick different settings depending on the profile and change the profile depending on the settings listed. If there's an overlap, pick a sane default unless the user forces a profile using profile:a. If forced and the samplerate isn't supported - error out and let the user know. For decoding set the profile via the demuxer. There doesn't have to be just one demuxer for a codec. If there are major changes in how you parse profiles go ahead and make a new one which sets the codec ID and the profile. In case the differences between the profiles are big yet there's still one decoder which will decode both, we can also call the encoder something different. In the case of Dirac and VC2, which are both AV_CODEC_ID_DIRAC, we just call the vc2 encoder "vc2". > _______________________________________________ > 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