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

Not selecting the correct video track for the video exported from Pixel8 captured motion JPEG #1051

Closed
1 task
cucumbersw opened this issue Jan 30, 2024 · 5 comments
Assignees

Comments

@cucumbersw
Copy link

cucumbersw commented Jan 30, 2024

Version

Media3 1.2.1

More version details

No response

Devices that reproduce the issue

Pixel8 Android 14, Galaxy S23 Android 14

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Yes

Reproduction steps

  1. Use Pixel8 to capture a motion-jpeg and extract the video from it, which will composed by 2 video tracks(and 2 meta tracks), both hevc, with one normal track has smaller resolution but another one has better resolution and contains only ONE frame. (Pixel8 google photos bug?)
  2. Use exoplayer to play it and you will find the default video track selected to play is the one which has better solution but only contains one frame
  3. The behavior cannot be changed even using track selection parameters by set min framerate to a large number, e.g. 20

Analyze:

  1. In Mp4Extractor.java, the frameRate is calculated as
if (trackDurationUs > 0 && trackSampleTable.sampleCount > 1) {
   float frameRate = trackSampleTable.sampleCount / (trackDurationUs / 1000000f);
   formatBuilder.setFrameRate(frameRate);
}
  1. In DefaultTrackSelector.java, isWithinMinConstraints is decided by:
      isWithinMinConstraints =
          isSuitableForViewport
              && (format.width == Format.NO_VALUE || format.width >= parameters.minVideoWidth)
              && (format.height == Format.NO_VALUE || format.height >= parameters.minVideoHeight)
              && (format.frameRate == Format.NO_VALUE
                  || format.frameRate >= parameters.minVideoFrameRate)
              && (format.bitrate == Format.NO_VALUE
                  || format.bitrate >= parameters.minVideoBitrate);
  1. Thus, setting min framerate to some bigger number won't help to get the correct video track selected by default

Suggestion:

  1. maybe change frameRate calculation condition to
    if (trackDurationUs > 0 && trackSampleTable.sampleCount > 0)
    Since one frame is still a frame, dividing by the duration, we can always have a value.
  2. expose frame count in the format struct, and also allow user to set track selection param using the frame count value

Expected result

The correct video track should be selected by default

Actual result

The single-frame video track is selected by default

Media

Please use Pixel8 to capture a motion-jpeg and extract the video from it.

Bug Report

@icbaker
Copy link
Collaborator

icbaker commented Feb 9, 2024

Use Pixel8 to capture a motion-jpeg and extract the video from it, which will composed by 2 video tracks(and 2 meta tracks), both hevc, with one normal track has smaller resolution but another one has better resolution and contains only ONE frame. (Pixel8 google photos bug?)

Are you extracting the MP4 data yourself? If you pass the motion photo jpeg directly to ExoPlayer with a default configuration, then it should be handled by JpegExtractor and forwarded to MotionPhotoJpegExtractor which (since 1.2.1) ensures the first track (guaranteed to be the 'real' video track in this motion photo format) is marked with role=MAIN and all subsequent tracks are marked role=ALTERNATE - this results in DefaultTrackSelector picking the lower-res 'real' video track.

if (mp4Extractor == null) {
mp4Extractor = new Mp4Extractor(FLAG_MARK_FIRST_VIDEO_TRACK_WITH_MAIN_ROLE);
}


If you need to extract the MP4 data yourself then you should ensure you either only keep the first track and discard the rest, or use Mp4Extractor.FLAG_MARK_FIRST_VIDEO_TRACK_WITH_MAIN_ROLE when constructing your Mp4Extractor in order to get the same behaviour.


If you're already using JpegExtractor then it's possible there's a bug in this new code we just added.

@cucumbersw
Copy link
Author

I use GooglePhoto app's own extraction function to get the video mp4 out of the jpeg file. The extracted mp4 file contains 2 video tracks, with one track has only 1 frame.

Let's put the issue this way, regardless where the mp4 file comes from, what if we just use ExoPlayer to open a mp4 file that contains 2 video track, one track is normal but another one has only 1 frame but 'better' resolution? The current impl of the default track selector will select the 1-frame track, which might not be the good choice.

So, this is a bug to be fixed, right?

@icbaker
Copy link
Collaborator

icbaker commented Feb 12, 2024

I use GooglePhoto app's own extraction function to get the video mp4 out of the jpeg file.

Ah interesting - in that case then I agree with your original comment, this looks like a bug in the Photos app. The additional low-fps HEVC track is only used as an efficient way to encode some other images - these are only intended to be consumed in a 'photo' context, not a 'video' one, so when exporting the file as a video there's no reason to include this additional track.

I was able to repro the issue on a Pixel 7a with Photos 6.69.0.602445611, but only when disabling 'stabilization' during export (which imo further confirms this is a Photos bug, since I wouldn't expect disabling stabilization to change the number of tracks included in the video). I captured a motion photo and confirmed (by playing the jpeg with ExoPlayer) that it contains two HEVC tracks (and you can see the role= flags being added by JpegExtractor):

tracks [eventTime=0.04, mediaPos=0.00, window=0, period=0
  group [
    [X] Track:0, id=1, mimeType=video/hevc, codecs=hvc1.1.6.L153, res=1440x1080, color=BT709/Full range/SDR SMPTE 170M/8/8, fps=29.585798, roleFlags=[main], supported=YES
  ]
  group [
    [ ] Track:0, id=2, mimeType=video/hevc, codecs=hvc1.1.6.L153, res=2048x1536, color=BT709/Full range/SDR SMPTE 170M/8/8, roleFlags=[alt], supported=YES
  ]
  group [
    [ ] Track:0, id=null, mimeType=null, container=image/jpeg, supported=NO
  ]
  group [
    [ ] Track:0, id=3, mimeType=application/microvideo-meta-stream, supported=NO
  ]
  group [
    [ ] Track:0, id=4, mimeType=application/motionphoto-image-meta, supported=NO
  ]
  Metadata [
    Mp4Timestamp: creation time=3790577256, modification time=3790577256, timescale=10000
  ]
]

In the Photos app I then did:

  • 3-dots menu
  • Scroll sideways to 'Export'
  • Select Video and leave 'Keep stabilization' ticked

The resulting video has only a single video track and plays fine with ExoPlayer:

tracks [eventTime=0.05, mediaPos=0.00, window=0, period=0
  group [
    [X] Track:0, id=1, mimeType=video/avc, codecs=avc1.640029, res=1440x1080, color=BT709/Limited range/SDR SMPTE 170M/8/8, fps=28.960901, supported=YES
  ]
  Metadata [
    mdta: key=com.android.version, value=14
    Mp4Timestamp: creation time=3790577256, modification time=3790577362, timescale=10000
  ]
]

Then re-doing the export with 'keep stabilization' unticked, the resulting video has multiple video tracks and ExoPlayer selects the high-res, low-fps one:

tracks [eventTime=0.06, mediaPos=0.00, window=0, period=0
  group [
    [ ] Track:0, id=1, mimeType=video/hevc, codecs=hvc1.1.6.L153, res=1440x1080, color=BT709/Full range/SDR SMPTE 170M/8/8, fps=29.585798, supported=YES
  ]
  group [
    [X] Track:0, id=2, mimeType=video/hevc, codecs=hvc1.1.6.L153, res=2048x1536, color=BT709/Full range/SDR SMPTE 170M/8/8, supported=YES
  ]
  group [
    [ ] Track:0, id=3, mimeType=application/microvideo-meta-stream, supported=NO
  ]
  group [
    [ ] Track:0, id=4, mimeType=application/motionphoto-image-meta, supported=NO
  ]
  Metadata [
    Mp4Timestamp: creation time=3790577256, modification time=3790577256, timescale=10000
  ]
]

Thanks for flagging, I'll file an internal bug against the Photos team and see what they say.

I will keep this issue open for now while we discuss whether ExoPlayer should also add a workaround for files like this (since any fix in Photos export logic obviously won't affect the files already created with this problem). I think some options are roughly:

  1. Change track selection logic to prefer tracks with 'real' FPS over tracks with 'unset' FPS.
    • This might (?) have unintended consequences on other videos with multiple tracks, where they are all actually 'playable'.
    • This alone doesn't solve the problem, because this motion photo format does allow more than one frame in the 'extra' HEVC track, at which point we would revert to picking the 'high res' track - so for this to be a complete fix we would also need to update DefaultTrackSelector to prefer tracks with a higher frame rate (perhaps capped by the display's max refresh rate, like by default we already cap resolution at the display's size).
    • For this to work to select the 'low res, high fps' track in this case we would either need to put fps as a higher priority than resolution (which probably doesn't reflect user preference in the general case - I suspect most people would prefer 4k 60fps content vs 720p 120fps content), or we would need to encode some concept of a 'normal' or 'playable' frame rate into DefaultTrackSelector. Maybe about 20fps as a threshold (based on scanning through https://en.wikipedia.org/wiki/Frame_rate), see next bullet too.
  2. Indicate somehow (in Format?) that a track only has a single frame.
    • This isn't ideal because (as above) these tracks aren't guaranteed to only have a single frame.
  3. Update Mp4Extractor to add the role=MAIN/ALTERNATE flags by default for all files with multiple video tracks where the first track has a 'normal' frame rate (defined as >20fps?) and subsequent tracks all have a frame rate less than this threshold (or only a single frame).

We would also need to discuss whether we should keep the new change in JpegExtractor and this workaround if we add one. They aren't both required, but the JpegExtractor change isn't a workaround it's a 'normal' part of dealing with this motion photo format, whereas any of the changes discussed feel more like workarounds for broken/incomplete media.

@icbaker
Copy link
Collaborator

icbaker commented Feb 12, 2024

I filed internal b/324844971 against the Photos app for this issue.

copybara-service bot pushed a commit that referenced this issue Feb 28, 2024
This change aims to prioritise tracks that have a 'smooth enough for
video' frame rate, without always selecting the track with the highest
frame rate.

In particular MP4 files extracted from motion photos sometimes have two
HEVC tracks, with the higher-res one having a very low frame rate (not
intended for use in video playback). Before this change
`DefaultTrackSelector` would pick the low-fps, high-res track.

This change adds a somewhat arbitrary 10fps threshold for "smooth video
playback", meaning any tracks above this threshold are selected in
preference to tracks below it. Within the tracks above the threshold
other attributes are used to select the preferred track. We deliberately
don't pick the highest-fps track (over pixel count and bitrate), because
most users would prefer to see a 30fps 4k track over a 60fps 720p track.

This change also includes a test MP4 file, extracted from the existing
`jpeg/pixel-motion-photo-2-hevc-tracks.jpg` file by logging
`mp4StartPosition` in
[`MotionPhotoDescription.getMotionPhotoMetadata`](https://github.com/androidx/media/blob/b930b40a16c06318e43c81771fa2b1024bdb3f29/libraries/extractor/src/main/java/androidx/media3/extractor/jpeg/MotionPhotoDescription.java#L123)
and then using `dd`:

```
mp4StartPosition=2603594

$ dd if=jpeg/pixel-motion-photo-2-hevc-tracks.jpg \
    of=mp4/pixel-motion-photo-2-hevc-tracks.mp4 \
    bs=1 \
    skip=2603594
```

----

This solution is in addition to the `JpegMotionPhotoExtractor` change
made specifically for these two-track motion photos in
5266c71.
We will keep both changes, even though that change is not strictly
needed after this one, because adding the role flags helps to
communicate more clearly the intended usage of these tracks. This
change to consider FPS seems like a generally useful improvement to
`DefaultTrackSelector`, since it seems unlikely we would prefer a 5fps
video track over a 30fps one.

Issue: #1051
PiperOrigin-RevId: 611015459
copybara-service bot pushed a commit that referenced this issue Feb 28, 2024
A follow-up change will add the frame rate to the single-frame track.

Issue: #1051
PiperOrigin-RevId: 611018319
copybara-service bot pushed a commit to google/ExoPlayer that referenced this issue Feb 28, 2024
This change aims to prioritise tracks that have a 'smooth enough for
video' frame rate, without always selecting the track with the highest
frame rate.

In particular MP4 files extracted from motion photos sometimes have two
HEVC tracks, with the higher-res one having a very low frame rate (not
intended for use in video playback). Before this change
`DefaultTrackSelector` would pick the low-fps, high-res track.

This change adds a somewhat arbitrary 10fps threshold for "smooth video
playback", meaning any tracks above this threshold are selected in
preference to tracks below it. Within the tracks above the threshold
other attributes are used to select the preferred track. We deliberately
don't pick the highest-fps track (over pixel count and bitrate), because
most users would prefer to see a 30fps 4k track over a 60fps 720p track.

This change also includes a test MP4 file, extracted from the existing
`jpeg/pixel-motion-photo-2-hevc-tracks.jpg` file by logging
`mp4StartPosition` in
[`MotionPhotoDescription.getMotionPhotoMetadata`](https://github.com/androidx/media/blob/b930b40a16c06318e43c81771fa2b1024bdb3f29/libraries/extractor/src/main/java/androidx/media3/extractor/jpeg/MotionPhotoDescription.java#L123)
and then using `dd`:

```
mp4StartPosition=2603594

$ dd if=jpeg/pixel-motion-photo-2-hevc-tracks.jpg \
    of=mp4/pixel-motion-photo-2-hevc-tracks.mp4 \
    bs=1 \
    skip=2603594
```

----

This solution is in addition to the `JpegMotionPhotoExtractor` change
made specifically for these two-track motion photos in
146bf6f.
We will keep both changes, even though that change is not strictly
needed after this one, because adding the role flags helps to
communicate more clearly the intended usage of these tracks. This
change to consider FPS seems like a generally useful improvement to
`DefaultTrackSelector`, since it seems unlikely we would prefer a 5fps
video track over a 30fps one.

Issue: androidx/media#1051
PiperOrigin-RevId: 611015459
copybara-service bot pushed a commit to google/ExoPlayer that referenced this issue Feb 28, 2024
A follow-up change will add the frame rate to the single-frame track.

Issue: androidx/media#1051
PiperOrigin-RevId: 611018319
@icbaker icbaker closed this as completed Mar 5, 2024
@icbaker
Copy link
Collaborator

icbaker commented Mar 5, 2024

The commits above change the behaviour of DefaultTrackSelector to prefer tracks with a frame rate greater than 10fps, which makes ExoPlayer select the 'real' video track by default.

copybara-service bot pushed a commit that referenced this issue Mar 7, 2024
Issue: #1051
PiperOrigin-RevId: 613516802
copybara-service bot pushed a commit to google/ExoPlayer that referenced this issue Mar 7, 2024
SheenaChhabra pushed a commit that referenced this issue Apr 8, 2024
This change aims to prioritise tracks that have a 'smooth enough for
video' frame rate, without always selecting the track with the highest
frame rate.

In particular MP4 files extracted from motion photos sometimes have two
HEVC tracks, with the higher-res one having a very low frame rate (not
intended for use in video playback). Before this change
`DefaultTrackSelector` would pick the low-fps, high-res track.

This change adds a somewhat arbitrary 10fps threshold for "smooth video
playback", meaning any tracks above this threshold are selected in
preference to tracks below it. Within the tracks above the threshold
other attributes are used to select the preferred track. We deliberately
don't pick the highest-fps track (over pixel count and bitrate), because
most users would prefer to see a 30fps 4k track over a 60fps 720p track.

This change also includes a test MP4 file, extracted from the existing
`jpeg/pixel-motion-photo-2-hevc-tracks.jpg` file by logging
`mp4StartPosition` in
[`MotionPhotoDescription.getMotionPhotoMetadata`](https://github.com/androidx/media/blob/b930b40a16c06318e43c81771fa2b1024bdb3f29/libraries/extractor/src/main/java/androidx/media3/extractor/jpeg/MotionPhotoDescription.java#L123)
and then using `dd`:

```
mp4StartPosition=2603594

$ dd if=jpeg/pixel-motion-photo-2-hevc-tracks.jpg \
    of=mp4/pixel-motion-photo-2-hevc-tracks.mp4 \
    bs=1 \
    skip=2603594
```

----

This solution is in addition to the `JpegMotionPhotoExtractor` change
made specifically for these two-track motion photos in
5266c71.
We will keep both changes, even though that change is not strictly
needed after this one, because adding the role flags helps to
communicate more clearly the intended usage of these tracks. This
change to consider FPS seems like a generally useful improvement to
`DefaultTrackSelector`, since it seems unlikely we would prefer a 5fps
video track over a 30fps one.

Issue: #1051
PiperOrigin-RevId: 611015459
(cherry picked from commit c7e00b1)
SheenaChhabra pushed a commit that referenced this issue Apr 8, 2024
Issue: #1051
PiperOrigin-RevId: 613516802
(cherry picked from commit afacf2c)
l1068 pushed a commit to l1068org/media that referenced this issue Apr 15, 2024
This change aims to prioritise tracks that have a 'smooth enough for
video' frame rate, without always selecting the track with the highest
frame rate.

In particular MP4 files extracted from motion photos sometimes have two
HEVC tracks, with the higher-res one having a very low frame rate (not
intended for use in video playback). Before this change
`DefaultTrackSelector` would pick the low-fps, high-res track.

This change adds a somewhat arbitrary 10fps threshold for "smooth video
playback", meaning any tracks above this threshold are selected in
preference to tracks below it. Within the tracks above the threshold
other attributes are used to select the preferred track. We deliberately
don't pick the highest-fps track (over pixel count and bitrate), because
most users would prefer to see a 30fps 4k track over a 60fps 720p track.

This change also includes a test MP4 file, extracted from the existing
`jpeg/pixel-motion-photo-2-hevc-tracks.jpg` file by logging
`mp4StartPosition` in
[`MotionPhotoDescription.getMotionPhotoMetadata`](https://github.com/androidx/media/blob/b930b40a16c06318e43c81771fa2b1024bdb3f29/libraries/extractor/src/main/java/androidx/media3/extractor/jpeg/MotionPhotoDescription.java#L123)
and then using `dd`:

```
mp4StartPosition=2603594

$ dd if=jpeg/pixel-motion-photo-2-hevc-tracks.jpg \
    of=mp4/pixel-motion-photo-2-hevc-tracks.mp4 \
    bs=1 \
    skip=2603594
```

----

This solution is in addition to the `JpegMotionPhotoExtractor` change
made specifically for these two-track motion photos in
androidx@5266c71.
We will keep both changes, even though that change is not strictly
needed after this one, because adding the role flags helps to
communicate more clearly the intended usage of these tracks. This
change to consider FPS seems like a generally useful improvement to
`DefaultTrackSelector`, since it seems unlikely we would prefer a 5fps
video track over a 30fps one.

Issue: androidx#1051
PiperOrigin-RevId: 611015459
(cherry picked from commit c7e00b1)
l1068 pushed a commit to l1068org/media that referenced this issue Apr 15, 2024
Issue: androidx#1051
PiperOrigin-RevId: 613516802
(cherry picked from commit afacf2c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants