VideoDecoder texture questions

47 views
Skip to first unread message

Michael Morrison

unread,
Apr 15, 2015, 8:37:21 PM4/15/15
to native-cli...@googlegroups.com
I have a couple of questions about implementation details in the files video_decoder_resource.cc and video_decoder_shim.cc.  Since this is in the context of using NaCL pp::VideoDecode I hope this is the right place to ask them.

VideoDecoderResource::OnPluginMsgRequestTextures creates textures using MIN_ and MAG_ FILTER of GL_NEAREST.  Is there a reason this is not using GL_LINEAR?  Using GL_NEAREST creates problems for plugins using p::VideoDecoder that want to resample the output texture and/or resize it (as the quality is poor with NEAREST).  Could this be changed to GL_LINEAR?  Our NaCL module is currently forced to reset the filters to GL_LINEAR every time it binds a texure received from decode.

VideoDecoderShim::SendPictures uploads the new texture image to an existing texture map using TexImage2D.  When talking directly to OpenGL, it should be faster in almost every case to create the image with the proper format and then use TexSubImage2D to upload the data.  This may be the case also when using Angle.  Is there a specific reason this is using TexImage2D?

Thanks.

Bill Budge

unread,
Apr 16, 2015, 9:54:16 AM4/16/15
to native-cli...@googlegroups.com
Great questions.

Apparently, GL_LINEAR can be used, and it does seem more appropriate. Here's an example in chromium:

Could you file an issue to track the work to change this?

As for your second question, TexImage2D is used because it's simpler. This online performance test suggests the difference is small but that you are right, TexSubImage2D is faster:


I tried to change the code but couldn't get it to work. If you would like to submit a patch, I'd be happy to look at it.

Michael Morrison

unread,
Apr 16, 2015, 2:40:50 PM4/16/15
to native-cli...@googlegroups.com
Hi Bill.  The bug website says it is readonly right now so I am unable to file a bug ATM.

I made the following changes locally, and things seem to work.  I would be happy to file a patch when I'm able.

video_decoder_resource.cc:
      gles2_impl_->BindTexture(texture_target, texture_ids[i]);
      gles2_impl_->TexParameteri(
>>          texture_target, GL_TEXTURE_MIN_FILTER, GL_LINEAR );
      gles2_impl_->TexParameteri(
>>          texture_target, GL_TEXTURE_MAG_FILTER, GL_LINEAR );
      gles2_impl_->TexParameterf(
          texture_target, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
      gles2_impl_->TexParameterf(
          texture_target, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);

      if (texture_target == GL_TEXTURE_2D) {
        gles2_impl_->TexImage2D(texture_target,
                                0,
>>                                GL_BGRA_EXT,
                                size.width,
                                size.height,
                                0,
>>                                GL_BGRA_EXT,
                                GL_UNSIGNED_BYTE,
                                NULL);
      }
 
In the above changes it is not clear to me whether there needs to be an #ifdef for OS_ANDROID or not when using BGRA.

video_decoder_shim.cc:
replaced TexSubImage with:

>>    gles2->TexSubImage2D(GL_TEXTURE_2D,
                         0,     /* level */
                         0,0,   /* x,y offset */
                         texture_size_.width(),
                         texture_size_.height(),
                         GL_BGRA_EXT,
                         GL_UNSIGNED_BYTE,
                         &frame->argb_pixels.front());

Michael Morrison

unread,
Apr 16, 2015, 3:57:48 PM4/16/15
to native-cli...@googlegroups.com
Filed issues: 
 Issue 477734:
 Issue 477737:

Bill Budge

unread,
Apr 17, 2015, 1:40:24 PM4/17/15
to native-cli...@googlegroups.com
Hi Michael,

The filtering change seems fine. However, the TexSubImage2D change doesn't work for me. The pictures don't appear, and this error is logged:

[68836:1287:0417/103850:ERROR:texture_manager.cc(1513)] [GroupMarkerNotSet(crbug.com/242999)!:D098103A877F0000]GL ERROR :GL_INVALID_OPERATION : glTexSubImage2D: format != internalformat

Michael Morrison

unread,
Apr 17, 2015, 3:23:41 PM4/17/15
to native-cli...@googlegroups.com
Bill,

The changes below are also necessary in video_decoder_resource.cc, to change the base texture internal format that is allocated to GL_BGRA_EXT instead of GL_RGBA.  This caused that error to go away for me.
Reply all
Reply to author
Forward
0 new messages