Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply Cobalt customizations to updated Chromium media #2854

Conversation

osagie98
Copy link
Contributor

@osagie98 osagie98 commented Apr 6, 2024

This pull request modifies the //media directory to allow it to build with Cobalt. These modifications are guarded with `#if defined(STARBOARD).

This pull request also includes the following changes:

  1. Applies upstream Chromium changes to support IAMF audio. These changes are not guarded.
  2. Creates new build targets for //ui/gfx and includes them in the build.

Remaining gaps:

  • MediaLog is stubbed out. We should remove the customization and enable it. Tracked in b/230916218
  • //skia and //ui/gfx have minor customizations that allow them to build with Cobalt. Most or all of these should be removed once //third_party/skia is updated to match Chromium.
  • PipelineStatusCB is planned to be replaced with PipelineStatusCallback. We should update Cobalt media to use only PipelineStatusCallback. Tracked in b/335673619

b/319697087

@osagie98
Copy link
Contributor Author

osagie98 commented Apr 11, 2024

Media now builds and links, but doesn't run. I'll list notes I jotted down during customization, I'll need to resolve most of these before it's complete:

  • Noticed in media/formats/mp4/track_run_iterator.cc line 485 that the Chromecast buildflag had been changed to #if 0 in the base update feature branch, check later if this is a build issue

  • ReadCB in media/base/demuxer_stream.h is an opportunity to remove a customization (I later removed it)

  • media/base/media_log_record.h, params was changed from base::DictionaryValue to base::Value. Now changed in the media update to base::Value::Dict. See if this may cause an issue.

  • Same in media/base/media_serializers.h

  • In media_serializers.h 333, check if SetKey needs to be changed to Set, among others in that file
    media/base/supported_types.cc has a lot of deleted hevc code, find out if it was moved

  • New customizations have been added in the base/net update. Find these.

  • Check why these lines https://github.com/youtube/cobalt/blame/960db55898df0c3c8676af8db98e3a7c1f8bd123/media/base/stream_parser_buffer.h#L66-L68 in stream parser buffer are excluded from the build. I removed this exclusion.

  • Check return value of ChunkDemuxer::RunSegmentParserLoop, it can return kSuccessHasMoreData, does this need to be handled? (I later added loops to run this function)

  • Do we update media_source_demuxer.cc/media2_sandbox.cc?

  • In sbplayer_pipeline, ondemuxerstreamread() no longer takes a reference to a vector of buffers, and instead makes a copy. Is this a problem?

@osagie98 osagie98 marked this pull request as ready for review April 12, 2024 19:32
@osagie98 osagie98 force-pushed the update-media-customizations branch from f5fe786 to 84df2e1 Compare April 15, 2024 21:00
Copy link
Contributor

@xiaomings xiaomings left a comment

Choose a reason for hiding this comment

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

Please update the primary commit message of the PR to include more details.

cobalt/media/base/metrics_provider.cc Outdated Show resolved Hide resolved
cobalt/media/base/metrics_provider.h Outdated Show resolved Hide resolved
media/base/demuxer.h Outdated Show resolved Hide resolved
media/base/supported_types.cc Show resolved Hide resolved
@osagie98 osagie98 changed the title Update media customizations Build updated media with Cobalt Apr 16, 2024
Change-Id: I94890e32576b62ff8dabfe6631541bed27c2e237
Change-Id: Ib6b6a47b36a8d8f20957253a627ddd24fd2f85d8
Change-Id: Iab0d473da2bf010906e23b615dd7a0f26171502b
Change-Id: If8f31e475f395e9194e9a3895cecc2282d2411b1
Change-Id: I95b6651afb168c610bdbf55e51107a34f09907ff
Change-Id: I2672833a74541c0ef93e25eb60007d62cf0af2e1
Change-Id: Ib11137f74cb717c590093ec8f9c74c1d0046f5a8
Also creates a //ui/gfx:gfx GN target.

Change-Id: I2d7c38fef58d9e0c81582b1e0af21a11b64f83af
Change-Id: I8cf034408e682c0b5a2118b27a182b2e8132abe5
Change-Id: I71f1cbf9cf6c27b569841cb01c0435c72629e341
@osagie98 osagie98 changed the title Build updated media with Cobalt Apply Cobalt customizations to updated media Apr 16, 2024
Apply customizations to //media so that Cobalt can build.

b/319697087
@osagie98 osagie98 force-pushed the update-media-customizations branch from be64058 to 2bb2c83 Compare April 16, 2024 21:24
media/base/audio_codecs.cc Show resolved Hide resolved
media/base/media_log.h Show resolved Hide resolved
media/base/media_util.cc Show resolved Hide resolved
media/base/status.h Show resolved Hide resolved
media/filters/chunk_demuxer.cc Outdated Show resolved Hide resolved
Fixes a build error with the media_sandbox target.
Change-Id: I0da38161e11dc0118a12ece369f885239f000bc5
Change-Id: I08e32d143799028052d6850ff364338840ed53d5
@osagie98 osagie98 changed the title Apply Cobalt customizations to updated media Apply Cobalt customizations to updated Chromium media Apr 18, 2024
@osagie98 osagie98 merged commit 42cd30e into youtube:feature/media-upstream-update Apr 18, 2024
258 of 289 checks passed
@osagie98 osagie98 deleted the update-media-customizations branch April 18, 2024 19:07
osagie98 added a commit that referenced this pull request Apr 18, 2024
Merges the feature/media-upstream-update branch to main.

See the major commits to feature/media-upstream-update:

- Import `//skia` from Chromium m114:
#2816
- Update `//ui/gfx` to match Chromium m114:
#2844
- Update `//media` to match Chromium m114:
#2960
- Apply customizations to updated dependencies to build with Cobalt:
#2854

b/319697087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants