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

Implement an FfmpegVideoRenderer #7079

Closed
wants to merge 1 commit into from
Closed

Implement an FfmpegVideoRenderer #7079

wants to merge 1 commit into from

Conversation

haohaozaici
Copy link

@haohaozaici haohaozaici commented Mar 13, 2020

I implemented a simple ffmpeg video extension that can currently be used to decode H.264 format videos.

Briefly explain what I did.

  1. create FfmpegVideoRenderer same as Libgav1VideoRenderer
  2. create FfmpegVideoDecoder like Gav1Decoder but decode method like FfmpegDecoder, then provide getExtraData() method for h264 type.
  3. Modify the ffmpegDecode jni method from the ffmpeg_jni.cc file. In decode method, i separated the send packet method and passed inputBuffer.timeUs as AVPacket.pts. Use the receive frame method to receive the decoded data and copy it to the outputBuffer.
  4. create FFmpegRenderersFactory to build custom VideoRenderers.

If anyone wants to use it or help with this extension, please refer to the README.

If I have any mistakes or features that need to be added, please feel free to raise them and I will try my best to modify them.

The features currently supported are very limited, including:

  • video supported codec: only H.264
  • audio supported codec: same as original extension
  • supported surface type: video_decoder_gl_surface_view

There are many things to do next, including:

  • Support other surface types
  • Fix possible issues
  • Organize the code
  • Video Decoder support Format.rotationDegrees
  • Support other codecs

@tonihei
Copy link
Collaborator

tonihei commented Mar 13, 2020

Thanks for filing the pull request!

@ojw28 Do you want to take a look? Feel free to reassign if not.

ojw28 added a commit that referenced this pull request Mar 19, 2020
The restriction that these classes only work with SimpleDecoders
is unnecessary. An FfmpegVideoRenderer will not be able to use a
SimpleDecoder, because the SimpleDecoder assumption that each input
buffer can be decoded immediately into a corresponding output is
not true for all video codecs that Ffmpeg supports (e.g., H264 does
not have this property). Generalizing SimpleDecoderVideoRenderer to
DecoderVideoRenderer will allow FfmpegVideoRenderer to still use
the base class, without having to use a SimpleDecoder.

This is a preliminary change toward being able to merge a version
of #7079.

Issue: #2159
PiperOrigin-RevId: 301412344
@ojw28
Copy link
Contributor

ojw28 commented Mar 19, 2020

Hi @haohaozaici - Thanks for sending this! I would like to merge this, but in order to do so I think we need it to modify the existing FFmpeg extension, rather than create a new one. To help, I've submitted a number of preliminary changes that help to get things in place. See the commits referenced from #2159 for details.

It would be great if you could have a go at reworking this pull request to slot into the existing FFmpeg extension now that these changes are in place. Roughly, I think you need to:

  1. Merge your JNI changes into the existing ffmpeg_jni.cc file.
  2. Rename FfmpegDecoder to FfmpegAudioDecoder.
  3. Add your new FfmpegVideoDecoder class.
  4. Merge your FfmpegVideoRenderer changes into the FfmpegVideoRenderer class, which is currently empty.

Notes:

  1. 818925d makes it so that FfmpegVideoRenderer will be automatically picked up and used by DefaultRenderersFactory if using EXTENSION_RENDERER_MODE_PREFER or EXTENSION_RENDERER_MODE_ON. This means your FFmpegRenderersFactory class is no longer required.
  2. The necessary change in FfmpegLibrary has already been made, in b7a5ace.
  3. You no longer need separate FfmpegAudioDecoderException and FfmpegVideoDecoderException classes. You can use the common FfmpegDecoderException for both.

Please let us know if you plan to take a look at this! Thanks.

@haohaozaici
Copy link
Author

Thank you very much for your suggestion, I will modify it as soon as possible.

@haohaozaici
Copy link
Author

Thank you for making a lot of changes. I completed the changes based on your suggestions, but I accidentally performed the wrong operation when I entered the git command. Then I deleted the repository of my fork and re-fork a copy. Now I don't know how to proceed. I'm really sorry, do I need to resubmit a pull request, or is there any other way to fix this?

@ojw28
Copy link
Contributor

ojw28 commented Mar 23, 2020

Sending a new pull request is fine, if that's easiest for you!

@haohaozaici
Copy link
Author

haohaozaici commented Mar 23, 2020

Thank you very much!
I send a new pull request #7132

@ojw28
Copy link
Contributor

ojw28 commented Mar 23, 2020

Superseded by #7132.

@ojw28 ojw28 closed this Mar 23, 2020
ojw28 added a commit that referenced this pull request May 5, 2020
- Fix DecoderAudioRenderer to re-init codec if the DRM session changes.
- Add canKeepCodec to DecoderVideoRenderer. Previously it was assumed
  that the decoder could be re-used, but this will not be true in all
  cases for FfmpegVideoRenderer.

Issue: #7079
PiperOrigin-RevId: 309935278
@google google locked and limited conversation to collaborators May 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants