Add support for parsing chapters from MKV files

[tymmesyde]

Hi,
this include changes to support parsing MKV chapters using the recent changes to introduce metadata Chapter interface
This is already implemented for MP4: Nero & QuickTime
Used this doc as reference: https://www.matroska.org/technical/elements.html

Added sample_with_chapters.mkv for mkvSample_withChapters test
Generated with:

;FFMETADATA1
[CHAPTER]
TIMEBASE=1/1000
START=0
END=1999
title=Chapter 1

[CHAPTER]
TIMEBASE=1/1000
START=2000
END=4999
title=Chapter 2

ffmpeg \                                                                                       
  -f lavfi -i testsrc=duration=5:size=320x240:rate=30 \
  -f lavfi -i sine=frequency=440:duration=5 \
  -i chapters.txt \
  -map 0:v -map 1:a -map_chapters 2 \
  -c:v libx264 -c:a aac \
  sample_with_chapters.mkv

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[nift4]

How many of the chapter features does this support?
https://github.com/hubblec4/Matroska-Playback/blob/master/src/PlayerOverview.md

[icbaker]

The tests seem to fail at the moment with

deduplicateConsecutiveFormats=false so TrackOutput must receive at least one sampleMetadata() call between format() calls.

Please can you get them passing?

[tymmesyde]

The tests seem to fail at the moment with

deduplicateConsecutiveFormats=false so TrackOutput must receive at least one sampleMetadata() call between format() calls.

Please can you get them passing?

They should pass now sorry, i guess i didn't actually run them after setting WRITE_TO_LOCAL
This is what was causing them to fail: 81efac3#diff-e2187c4432661736c918e5ca266afbeec7c6709b184729dec6a4aa613abf0c0aL968, seems like this should only be called by the logic that handle the samples?

[tymmesyde]

How many of the chapter features does this support? https://github.com/hubblec4/Matroska-Playback/blob/master/src/PlayerOverview.md

Only Basic Chapters, and it only expose the title (ChapString) and timeStart/timeEnd for the Chapter interface

[icbaker]

@nift4 Since the media3 Chapter interface is new, we have an opportunity to change it before it gets included in a release.

In particular I'm wondering whether to add a concept of 'disabled/enable' and/or 'hidden/shown' to the Chapter interface to reflect these fields from the Matroska spec. I think all other features we might want to support can be safely added later through additional interface methods, possibly with default implementations, e.g. multiple titles with associated languages. But the isDisabled/isHidden methods probably need to be on the interface from the start (so apps know to check them).

It sems like a 'hidden' chapter should still be played, but just not shown to a user in a Table-of-Contents (i.e. this can be implemented "app side").

Whereas a 'disabled' chapter should be skipped during playback. I think we are unlikely to add automatic support for this part in the Player (at least not for a while). It could probably still be implemented app-side using player messages or similar.

Do you think it makes sense to add these methods to the 'generic' Chapter interface? They would return 'enabled' and 'not hidden' for other sources of chapters that don't encode the same concept (e.g. MP4 Nero, or ID3).

[nift4]

@icbaker Yes, I think that's a good idea.

[icbaker]

@tymmesyde Would it be possible to open a new PR from an individual-owned fork? We can't push changes to organization-owned forks like this one unless we have collaborator access. If that's not possible then we can still merge this PR but it will result in an 'evil' merge. See more info here: https://github.com/androidx/media/blob/release/CONTRIBUTING.md#push-access-to-pr-branches

[tymmesyde]

sure, here you go: #3144

[tymmesyde]

Closed in favor of #3144