[FFmpeg-devel] matroska encoder option -- force new Cluster on keyframe

[Bernie Habermeier]

On Jul 20, 2013, at 1:54 PM, Michael Niedermayer wrote:

> On Fri, Jul 19, 2013 at 05:11:54PM -0700, Bernie Habermeier wrote:
>> I'm working on implementing a video player that would benefit from a tiny optional change to the matroska encoder in ffmpeg.   I propose to add a command line option that generates a valid matroska file with the added constraint that each keyframe gets a new cluster.   I'm writing to ask if folks think that adding this option to ffmpeg is acceptable before I submit such a patch.
> 
> this could cause problems (significant overhead) with intra only
> streams
> It possibly should thus be limited to keyframes not preceded by
> another keyfrane
> 
> But in principle the feature sounds like a good idea

Agree that this would be really kind of bad for intra only streams, but to be honest the current implementation in matroskaenc.c creates a CuePoint for each video key frame, and arguably, it's a little bit insane to have a CuePoint entry for every single video frame in a movie.  If you're coming from the point of view that this would impact overhead, you've already lost the battle on that front.  Though I concede that you don't HAVE to read the CuePoint table to play a movie, whereas you DO have to seek through Clusters.

However, I'd argue that given this flag is entirely optional -- something that you have to turn on, I would allow the user to make their choice to turn it or or not without protecting them from themselves regarding the additional overhead.  That would be preferable in my mind than writing logic that violates the stipulation of the argument in question.  If the user expects a new Cluster to start for every CuePoint, then by golly -- let it be so.  As an end-user I'd rather know that it's doing what I asked it to, regardless if I'm asking it to do something questionable.  

Even if we decided to add some kind of protection against "excessive overhead", I'm not sure where exactly I'd hardcode the line of protection.  A GOP of size 1 is the most extreme, but what about 2, 3 or 5?  

This is why I think this should be an option, and one that you'd only turn on if you thought this was something that makes sense to you.  Overall, when you transcode a movie, you have to make a lot of tradeoffs about size and quality.  This option just allows you to make an additional decision regarding seek time efficiency -- for cases where you absolutely care about this -- perhaps on mobile devices.

>> Proposed option name: -force_new_cluster_on_key_frame
>> or perhaps: -new_cluster_on_key_frame
>> 
>> Though I'd be happy to have it be something different (assuming people are OK with the idea).
>> 
> 
>> It might be better to think about it in terms of CuePoints.  Nothing says that you have to have a CuePoint for each keyframe in the stream.  Perhaps another way to describe the above is via CuePoints.  So instead of saying that for each keyframe we create a new cluster, you'd say you create a new cluster for each CuePoint.  I believe currently each keyframe gets an entry in the Cues index.
> 
> agree

In this case I'd say the option might be called:

	-new_cluster_on_cuepoint

Dropping the 'force', to make the option not as long :)

So, if I make a patch for this, who would review / approve it?  Does this need more discussion?  


Thanks,
Bernie