[FFmpeg-devel] [PATCH] mpeg4video: Add support for MPEG-4 Simple Studio Profile.

Michael Niedermayer michael at niedermayer.cc
Sat Jan 27 02:33:18 EET 2018


On Fri, Jan 26, 2018 at 08:04:39PM +0000, Kieran Kunhya wrote:
> On Mon, Jan 22, 2018 at 2:07 AM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
> 
> > On Sun, Jan 21, 2018 at 12:37:21PM +0000, Kieran Kunhya wrote:
> > > On Mon, Jan 1, 2018 at 7:01 PM, Michael Niedermayer
> > <michael at niedermayer.cc>
> > > wrote:
> > >
> > > > Hi
> > >
> > >
> >
> > > Patch updated.
> > > Some of the review comments I decided not to implement in order to keep
> > > closer to the spec.
> >
> > honestly, this reasoning makes no sense to me.
> > that spec does IIRC not even fully describe some parts in DPCM
> >
> > also all existing mpeg&h26x code we have was designed for speed
> > basically ignoring if its close to some spec implementation style or not
> >
> > I would prefer we try to make our code as good as we can and not try to
> > be similar to some specification or reference implementation.
> > except for maybe some niche codecs where performance totally doesnt matter
> >
> 
> It does describe DPCM, just in an odd way.

The specification i read did not specify DPCM in a way one could implement
aka the description was not complete.

and it also was not understandable to you as you asked on IRC about it ages ago

Jän 22 03:12:54 <kierank>       michaelni: don't know if I am missing something here but does it say anywhere what the length of dpcm_residual is?
Jän 22 03:41:31 <kierank>       I'm guessing it should read 8-12?
Jän 22 03:41:41 <kierank>       in 14496-2
...
Dez 23 01:55:30 <kierank>       atomnuker: any idea how to calculate the length of dpcm_residual
Dez 23 01:57:19 <kierank>       it just says "dpcm_residual: This is an unsigned integer that indicates the value of a DPCM residual"
Dez 23 01:58:08 <nevcairiel>    shouldnt the definiton of "uimsbf" tell you how to parse those values
Dez 23 01:58:25 <kierank>       sure, but doesn't explain the 4-12
Dez 23 01:58:44 <nevcairiel>    how did you figure out the length of say block_mean. its 8-12 uimsbf
Dez 23 01:59:10 <kierank>       block mean is the average of the block and the supported bitdepths are 8-12
Dez 23 02:01:31 <nevcairiel>    so basically the format syntax is just bonkers
Dez 23 02:10:12 <kierank>       there must be something I'm missing

If the spec makes sense to you now, please explain because it made no sense
to me last i looked.
I genuinely do not understand what it meant


> I am going to implement your ricing changes but for the record your passive
> aggressiveness to many developers is awful.

i appologize if something i said sounded passive aggerssive, it was not intended
to.


> Please learn to compromise.

you refer to myself pushing towards a optimial implementation and not the spec?
as in " I would prefer we try to make our code as good as we can and not try to
be similar to some specification or reference implementation."
?

Dont you agree that this is the better choice for teh codebase even though yes
it is certainly more work ?


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180127/8c8e494e/attachment.sig>


More information about the ffmpeg-devel mailing list