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

Set pixel aspect ratio from output media format if set #1371

Merged
merged 4 commits into from
Aug 30, 2024

Conversation

dsparano
Copy link
Contributor

Problem
If a video effect is applied to the video, or an "enhancement" such as LCEVC, is applied, the output video may look distorted if the pixel aspect ratio was changed in the post-processing stage.

Root cause
The output pixel aspect ratio is taken from the input Format object, which is a property of the stream and not of the video decoding and post processing applied. Note that this has similarity with what was addressed in #1025.

Proposed solution
If pixel aspect ratio is set in the output MediaFormat object it is taken from there, since it may have been set in the post processing stage, otherwise set it from the Format object as per current behaviour.

@dsparano
Copy link
Contributor Author

Hi @microkatz have you had the chance to review this?

@dsparano
Copy link
Contributor Author

Rebased after conflict from main.

@dsparano
Copy link
Contributor Author

Hi @microkatz coming back here after a while, this change looks pretty straightforward, any reason for not merging it?

@microkatz
Copy link
Contributor

microkatz commented Aug 27, 2024

Apologies for the delay @dsparano but thank you for following up!

Would you be able to add a unit test for your change? You should be able to add a test in MediaCodecVideoRendererTest modeled after render_sendsVideoSizeChangeWithCurrentFormatValues.

I prototyped a unit test with your proposed code change so I know its possible.

This also highlighted some bugs in your proposed code.
1)Although the field pixelWidthHeightRatio is a float, because the two mediaFormat MediaFormat.KEY_PIXEL_ASPECT_RATIO_WIDTH and MediaFormat.KEY_PIXEL_ASPECT_RATIO_HEIGHT are integers, then if the ratio is less than 1.0f, it will round to 0.
2) The keys KEY_PIXEL_ASPECT_RATIO_WIDTH and KEY_PIXEL_ASPECT_RATIO_HEIGHT were introduced in API Level 30 and the minimum SDK level for the project is 21. So you'll need to add to the hasPixelAspectRatio boolean if Util.SDK_INT > 29.

Here was my rough prototyped test.
It required creating a subclass of ForwardingSynchronousMediaCodecAdapter in the test class that sets the PIXEL_ASPECT_RATIO_WIDTH and PIXEL_ASPECT_RATIO_HEIGHT values.

@Config(minSdk = 30)
@Test
  public void render_withMediaCodecAlteringPixelAspectRatioWidthHeight_sendsVideoSizeChangeWithMediaFormatValues() throws Exception {
    MediaCodecAdapter.Factory codecAdapterFactory =
        configuration ->
            new ForwardingSynchronousMediaCodecAdapterAlteringPixelAspectRatio(
                new SynchronousMediaCodecAdapter.Factory().createAdapter(configuration));
    MediaCodecVideoRenderer mediaCodecVideoRendererWithCustomAdapter =
        new MediaCodecVideoRenderer(
            ApplicationProvider.getApplicationContext(),
            codecAdapterFactory,
            mediaCodecSelector,
            /* allowedJoiningTimeMs= */ 0,
            /* enableDecoderFallback= */ false,
            /* eventHandler= */ new Handler(testMainLooper),
            /* eventListener= */ eventListener,
            /* maxDroppedFramesToNotify= */ 1) {
          @Override
          protected @Capabilities int supportsFormat(
              MediaCodecSelector mediaCodecSelector, Format format) {
            return RendererCapabilities.create(C.FORMAT_HANDLED);
          }
        };
    mediaCodecVideoRendererWithCustomAdapter.init(/* index= */ 0, PlayerId.UNSET, Clock.DEFAULT);
    surface = new Surface(new SurfaceTexture(/* texName= */ 0));
    mediaCodecVideoRendererWithCustomAdapter.handleMessage(Renderer.MSG_SET_VIDEO_OUTPUT, surface);

    FakeSampleStream fakeSampleStream =
        new FakeSampleStream(
            new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024),
            /* mediaSourceEventDispatcher= */ null,
            DrmSessionManager.DRM_UNSUPPORTED,
            new DrmSessionEventListener.EventDispatcher(),
            /* initialFormat= */ VIDEO_H264,
            ImmutableList.of(
                oneByteSample(/* timeUs= */ 0, C.BUFFER_FLAG_KEY_FRAME), END_OF_STREAM_ITEM));
    fakeSampleStream.writeData(/* startPositionUs= */ 0);
    mediaCodecVideoRendererWithCustomAdapter.enable(
        RendererConfiguration.DEFAULT,
        new Format[] {VIDEO_H264},
        fakeSampleStream,
        /* positionUs= */ 0,
        /* joining= */ false,
        /* mayRenderStartOfStream= */ true,
        /* startPositionUs= */ 0,
        /* offsetUs= */ 0,
        new MediaSource.MediaPeriodId(new Object()));
    mediaCodecVideoRendererWithCustomAdapter.setCurrentStreamFinal();
    mediaCodecVideoRendererWithCustomAdapter.start();

    int positionUs = 0;
    do {
      mediaCodecVideoRendererWithCustomAdapter.render(positionUs, SystemClock.elapsedRealtime() * 1000);
      positionUs += 10;
    } while (!mediaCodecVideoRendererWithCustomAdapter.isEnded());
    shadowOf(testMainLooper).idle();

    verify(eventListener)
        .onVideoSizeChanged(
            new VideoSize(
                VIDEO_H264.width,
                VIDEO_H264.height,
                VIDEO_H264.pixelWidthHeightRatio / 2));
  }

private static final class ForwardingSynchronousMediaCodecAdapterAlteringPixelAspectRatio
      extends ForwardingSynchronousMediaCodecAdapter {

    ForwardingSynchronousMediaCodecAdapterAlteringPixelAspectRatio(MediaCodecAdapter adapter) {
      super(adapter);
    }

@Override
    public MediaFormat getOutputFormat() {
      MediaFormat mediaFormat = adapter.getOutputFormat();
      if (SDK_INT > 29) {
        int pixelAspectRatioHeight = 1 << 30;
        int pixelAspectRatioWidth = (int) (0.5f * pixelAspectRatioHeight);
        mediaFormat.setInteger(MediaFormat.KEY_PIXEL_ASPECT_RATIO_WIDTH,
            pixelAspectRatioWidth);
        mediaFormat.setInteger(MediaFormat.KEY_PIXEL_ASPECT_RATIO_HEIGHT,
            pixelAspectRatioHeight);
      }
      return mediaFormat;
    }
  }

@dsparano
Copy link
Contributor Author

Thanks @microkatz, I think I've made the changes as per for your review and suggested code

@microkatz
Copy link
Contributor

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!

@microkatz microkatz force-pushed the dsparano-exo207 branch 2 times, most recently from c50c1d0 to b16ce6d Compare August 30, 2024 10:11
@copybara-service copybara-service bot merged commit 854566d into androidx:main Aug 30, 2024
1 check passed
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