[MPlayer-dev-eng] Lots of stuff for NUT

[Oded Shimon]

On Tue, Jan 03, 2006 at 09:34:32PM +0100, Michael Niedermayer wrote:
> On Tue, Jan 03, 2006 at 09:32:06PM +0200, Oded Shimon wrote:
> > 3. re-arrange main header
> 
> NO, this has not been disscussed, it maybe a good idea but until i hear
> your arguments i wont agree

With my frame code suggestion, you can make an invalid code with specifying 
just flags and headers, you can specify a safety net code by specifyingjust 
flags and headers, and I rearranged them what imo are those which have to 
be manually specified the most often (mul is on top, then sflag, then pts 
and then stream. Now that I think of it, maybe pts after stream.)

I can only guess that the reason you made it that way before was to allow 
"continued" codes which depended on the values of the last entry. You never 
really sepcified that this is defined behavior, and the only case it is 
useful is for frame code 'N' you want to put in the middle of a bigger 
frame code chunk, but to specify 'N' you need to atleast go upto tmp_mul, 
destroying the values you had "cached" from before.

> > 4. add coded_stream_flags
> 
> YES and may i suggest to change 
> stream_flags is "stream_flags[frame_code] | coded_stream_flags"
> to
> stream_flags is "stream_flags[frame_code] ^ coded_stream_flags"

Good idea...

> > 5. Change index
> 
> NO

Well, in reply to your "seperate patch" comment below, It's problematic for 
me for ex. to remove max_index_distance without changing index, same goes 
for removing global timebase without changing syncpoints... There is some 
cross dependant stuff, but I can probably atleast split it to most of the 
"cosmetic" changes and the significant changes.

> > 6. fix small bug in info_packets (no get_s())
> 
> YES
> 
> 
> > 7. per stream back_ptr and pts in syncpoints
> 
> NO, i agree with a single back_ptr and would abstain if you suggest per stream
> but iam against pts per stream

Well, what is it about it that bothers you? The overhead? I've already 
prooved that in high bitrate files, the overhead is irrelavent, and in low 
bitrate files, the frame headers are most of the overhead anyway. (If you 
do not agree with my tests please offer a better testing scenario)

> > 8. rename frame_startcode to syncpoint_startcode
> 
> YES
> 
> 
> > 9. add EOR and SOR stream flags
> 
> EOR: abstain
> SOR: NO

The reason for SOR is to clear the dts cache. I think we needed it back 
when we were using mts, maybe it's not necessary now, I'm trying to 
remember why we added it at all.

> > 10. keyframes with back_pts bigger than max_distance must be accompanied by 
> >     syncpoint
> 
> YES

I meant back_ptr* ofcourse

> > 11. define function "convert_ts"
> 
> YES
> 
> 
> > 12. Add "more_codes" frame header flag
> 
> u mean more_flags?

Ah, yes, I accidentaly said the same item here twice :)

> > 13. Change "monotone" to MN rule
> 
> YES
> 
> 
> > 14. max_pts is coded in same way as syncpoint's coded_pts
> 
> YES
> 
> 
> > 15. Bump draft date :)
> 
> YES
> 
> 
> > 16. Change goals slightly. :) Although actually with this new index I'm not 
> >     sure the index is even under 100kb. This needs checking.
> 
> NO, check first how large it is

OK, will do. If we find the index is not abnormally big, will you vote for 
syncpoint per stream pts?

> 17. remove index repeation possibility
> NO

Well, the spec never allowed it anyway, I just removed it from the goals.

> > Go over the patch, comment on it, I want to commit it, I want your OK's.
> 
> IMHO there are too many unrelated change the patch should be split, if
> its not split you can consider my (single) vote to be NO, the reason
> behind that is the cvs policy says to split unrelated things and that
> IMHO should apply here too

I generally hate seperating patches cause it makes both creating them and 
committing them annoying, but I'll try splitting this patch up...


Rich, could you go over the patch as well?

- ods15