Set pixel aspect ratio from output media format if set

[dsparano]

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]

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

[dsparano]

Rebased after conflict from main.

[dsparano]

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

[microkatz]

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]

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

[microkatz]

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]

Hello @dsparano,

Just to warn you, a regression in playback behavior was found subsequently to the introduction of this PR and we had to rollback this commit. This occurred here, 5282fe3. The rollback was pushed into main and will be released as part of 1.5.1.

The issue was that for some vertical videos, the provided pixel aspect ratios from a MediaCodec decoder were incorrect and should not have be applied at onOutputFormatChanged. We are still investigating the potential bug in the framework but will keep you posted on the progress.