Skip to content

fix: Allow video fourcc to be preserved in HLS#1523

Open
gmcgarry wants to merge 12 commits intoshaka-project:mainfrom
DolbyLaboratories:dolby/dont-adjust-video-codecs
Open

fix: Allow video fourcc to be preserved in HLS#1523
gmcgarry wants to merge 12 commits intoshaka-project:mainfrom
DolbyLaboratories:dolby/dont-adjust-video-codecs

Conversation

@gmcgarry
Copy link

This fix addresses #1494.

Currently, the behaviour is to substitute the 'dvhe' codec string with 'dvh1' in the HLS master playlist. This is to work-around a bug in the Apple native HLS parser which would fail on 'dvhe'. However, this substitution can cause incompatibility with other devices.

Discussions between Dolby and Apple have confirmed that 'dvhe' streams are supported and this is a bug in the low-level media pipeline. This bug is only present in QuickTime and the native Safari player. Apple will fix in a future update.

Meantime, this patch introduces a new command-line flag to strictly preserve the original video fourcc. The --strict_codecs_signaling flag will disable the substitution logic. The semantics of this option preserve the existing default behaviour of the packager.

As a part of the patch, a broader set of Dolby Vision tests and test material is included. The test material comes from Dolby's Reference Media Kit and licensed under Creative Commons.

Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

@joeyparrish
Copy link
Member

Looks like a linter failure:

https://github.com/shaka-project/shaka-packager/actions/runs/19561653238/job/56069813390?pr=1523

diff --git a/packager/app/packager_main.cc b/packager/app/packager_main.cc
index 371cd90..a15d189 100644
--- a/packager/app/packager_main.cc
+++ b/packager/app/packager_main.cc
@@ -544,7 +544,8 @@ std::optional<PackagingParams> GetPackagingParams() {
       absl::GetFlag(FLAGS_hls_media_sequence_number);
   hls_params.start_time_offset = absl::GetFlag(FLAGS_hls_start_time_offset);
   hls_params.create_session_keys = absl::GetFlag(FLAGS_create_session_keys);
-  hls_params.strict_codecs_signaling = absl::GetFlag(FLAGS_strict_codecs_signaling);
+  hls_params.strict_codecs_signaling =
+      absl::GetFlag(FLAGS_strict_codecs_signaling);
 
   TestParams& test_params = packaging_params.test_params;
   test_params.dump_stream_info = absl::GetFlag(FLAGS_dump_stream_info);


Code style is not correct. Please run git clang-format --style Chromium --binary /usr/bin/clang-format 5776b0b60d52c41732279e17fa2388d72d5a606d.

@joeyparrish
Copy link
Member

I tried to fix the linter error for you, but I was denied access. I suspect you have not checked the box that allows maintainers to make edits to your PR. Just let us know when you've pushed a commit to fix the linter.

@gmcgarry
Copy link
Author

I tried to fix the linter error for you, but I was denied access. I suspect you have not checked the box that allows maintainers to make edits to your PR. Just let us know when you've pushed a commit to fix the linter.

Thanks for taking a look. I'll work through the failures.

According to GitHub:

Currently, the "Allow edits from maintainers" feature is not supported for PRs created from forks owned by an organization rather than an individual.

@joeyparrish
Copy link
Member

https://github.com/shaka-project/shaka-packager/actions/runs/19584792038/job/56091585523?pr=1523

Running packager/tools/git/check_pylint.py produced:

packager/app/test/packager_test.py:1545:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1554:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1563:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1572:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1581:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1583:0: C0301: Line too long (96/80) (line-too-long)
packager/app/test/packager_test.py:1590:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1592:0: C0301: Line too long (96/80) (line-too-long)
packager/app/test/packager_test.py:1599:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1601:0: C0301: Line too long (96/80) (line-too-long)
packager/app/test/packager_test.py:1608:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1610:0: C0301: Line too long (96/80) (line-too-long)
packager/app/test/packager_test.py:1618:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1627:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1636:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1645:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1654:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1663:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1672:0: C0301: Line too long (85/80) (line-too-long)
packager/app/test/packager_test.py:1681:0: C0301: Line too long (85/80) (line-too-long)

@joeyparrish
Copy link
Member

According to GitHub:

Currently, the "Allow edits from maintainers" feature is not supported for PRs created from forks owned by an organization rather than an individual.

Oh, wow, I had no idea. Thanks!

@joeyparrish
Copy link
Member

These tests are failing on some jobs and not others. Could there be an uninitialized variable? I can't think of another good reason for the inconsistency.

https://github.com/shaka-project/shaka-packager/actions/runs/19586207575/job/56097637486?pr=1523

19: [----------] 6 tests from Codecs/MediaPlaylistCodecTest
19: [ RUN      ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/0
19: [       OK ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/0 (0 ms)
19: [ RUN      ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/1
19: /home/runner/work/shaka-packager/shaka-packager/packager/hls/base/media_playlist_unittest.cc:1188: Failure
19: Expected equality of these values:
19:   media_playlist_->codec()
19:     Which is: "avc3.4d401e"
19:   expected_output_codec
19:     Which is: "avc1.4d401e"
19: [  FAILED  ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/1, where GetParam() = ("avc3.4d401e", "avc1.4d401e") (0 ms)
19: [ RUN      ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/2
19: [       OK ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/2 (0 ms)
19: [ RUN      ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/3
19: /home/runner/work/shaka-packager/shaka-packager/packager/hls/base/media_playlist_unittest.cc:1188: Failure
19: Expected equality of these values:
19:   media_playlist_->codec()
19:     Which is: "hev1.2.4.L63.90"
19:   expected_output_codec
19:     Which is: "hvc1.2.4.L63.90"
19: [  FAILED  ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/3, where GetParam() = ("hev1.2.4.L63.90", "hvc1.2.4.L63.90") (0 ms)
19: [ RUN      ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/4
19: [       OK ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/4 (0 ms)
19: [ RUN      ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/5
19: /home/runner/work/shaka-packager/shaka-packager/packager/hls/base/media_playlist_unittest.cc:1188: Failure
19: Expected equality of these values:
19:   media_playlist_->codec()
19:     Which is: "dvhe.05.08"
19:   expected_output_codec
19:     Which is: "dvh1.05.08"
19: [  FAILED  ] Codecs/MediaPlaylistCodecTest.AdjustVideoCodec/5, where GetParam() = ("dvhe.05.08", "dvh1.05.08") (0 ms)
19: [----------] 6 tests from Codecs/MediaPlaylistCodecTest (0 ms total)

@gmcgarry gmcgarry requested a review from sr1990 November 27, 2025 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments