androidx
Add Dolby Vision Transcoding and Editing Support
This code contribution adds Dolby Vision transcoding and editing support to transformer.
@ybai001 I just wanted to update that due to other high priority work, it might take a few weeks to review this change. I appreciate your patience.
@ybai001 I just wanted to update that due to other high priority work, it might take a few weeks to review this change. I appreciate your patience.
Sure, understood.
We need to have some tests cases around these changes.
What can I do for this request? At least, I can provide test stream. If you mean I need to implement some test cases, could you guide me what kinds of test cases I should add. Previously, I added some DD+JOC and AC-4 test cases for my media3 contributions so that I am familiar with that process. But for transformer, this is my first contribution.
We need to have some tests cases around these changes.
What can I do for this request? At least, I can provide test stream. If you mean I need to implement some test cases, could you guide me what kinds of test cases I should add. Previously, I added some DD+JOC and AC-4 test cases for my media3 contributions so that I am familiar with that process. But for transformer, this is my first contribution.
We could include following tests:
- Transmuxing a dolby vision input file should preserve the dolby vision data. This could be a robolectric test. For this, you can refer tests in MediaItemExportTest.java
- Transcoding a normal H.264/H.265 to dolby vision. For this your can refer tests in TransformerEndToEndTest.java
- Editing a dolby vision input. For this your can refer tests in TransformerEndToEndTest.java
I would also suggest to run these scenario using Transformer demo app. This is to ensure that it works as expected.
We need to have some tests cases around these changes.
What can I do for this request? At least, I can provide test stream. If you mean I need to implement some test cases, could you guide me what kinds of test cases I should add. Previously, I added some DD+JOC and AC-4 test cases for my media3 contributions so that I am familiar with that process. But for transformer, this is my first contribution.
We could include following tests:
- Transmuxing a dolby vision input file should preserve the dolby vision data. This could be a robolectric test. For this, you can refer tests in MediaItemExportTest.java
- Transcoding a normal H.264/H.265 to dolby vision. For this your can refer tests in TransformerEndToEndTest.java
- Editing a dolby vision input. For this your can refer tests in TransformerEndToEndTest.java
I would also suggest to run these scenario using Transformer demo app. This is to ensure that it works as expected.
Hi, @SheenaChhabra, Thanks for your suggestion. As far as the three tests you mentioned,
- I checked the file MediaItemExportTest.java and there are several test cases for transmux. But all of them are general test cases. None of them is for specific video format, e.g. AV1. Could you help to clarify why we need to add test case for Dolby Vision but not for other video format? And which test case I should refer to? Instead, I find there is a case in https://github.com/androidx/media/blob/main/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/HdrEditingTest.java#L79. Should I create a case for Dolby Vision here?
- I have added the case as you mentioned. I created it based on this one (for AV1) https://github.com/androidx/media/blob/main/libraries/transformer/src/androidTest/java/androidx/media3/transformer/TransformerEndToEndTest.java#L1517
- I checked the file TransformerEndToEndTest.java. But none of them is related to HDR editing. Instead, I find there is a Dolby Vision editing case in https://github.com/androidx/media/blob/main/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/HdrEditingTest.java#L191. Is it enough for your requirement?
Finally, (1) I added export_transmuxDolbyVisionFile() in HdrEditingTest.java to match your requirement 1; (2) I added transcode_withOutputVideoMimeTypeDolbyVision_completesSuccessfully() in TransformerEndToEndTest.java to match your requirement 2; (3) I applied exist exportAndTranscode_dolbyVisionFile_whenHdrEditingIsSupported() and created exportAndTranscode_dolbyVisionFile_whenHdrEditingUnsupported_toneMapsOrThrows() in HdrEditingTest.java to match your requirement 3
If this solution is OK, I'll upload my code.
Looks good to me. @SheenaChhabra please approve when you think your comments have been resolved
@SheenaChhabra, I have updated code as your comments except resolving conflicts. Let me know whether there is any other comment from your side.
Overall changes are good but some of the comments are still not resolved. Once the comments are resolved/answered and the conflicts are resolved, we can take this forward.
I have updated the code as you requested and resolved the conflicts. Let's me know whether there is any other issue.
After resolving the merge conflict, I find the code base is updated to the latest one and I have to update my test code to match the latest one. (Some definitions are changed totally, e.g. MP4_ASSET. I'll refine my code and upload it.
@SheenaChhabra, I updated code based on latest code on main branch. I think I resolved all the conflicts. Let me know whether you have more questions. Sorry for tons of commits and comments today.
Thanks for making this change. I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!
Thanks. BTW, just want to let you know: Google can't push changes to this organization-owned forks because Dolby didn't grant you collaborator access. I have applied for it to our Dolby admin but she didn't agree with it. As result, you can merge this PR but it will result in an 'evil' merge. See details here: https://github.com/androidx/media/blob/release/CONTRIBUTING.md#push-access-to-pr-branches