openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.97k stars 2.55k forks source link

ofVideoPlayer linux / Error on buffer size for some videos when setting OF_PIXELS_NATIVE #3404

Open PetrosKataras opened 10 years ago

PetrosKataras commented 10 years ago

Hi,

I am with latest master under xubuntu 14.04.

If I set the player to use the default pixel format I get this error with a video captured from a GoPro camera :

[notice ] ofGstVideoPlayer: allocating with 860x646 I420 [ error ] ofGstVideoUtils: buffer_cb(): error on new buffer, buffer size: 834632!= init size: 833340 [ error ] ofGstUtils: gstHandleMessage(): embedded video playback halted for plugin, module qtdemux0 reported: GStreamer encountered a general stream error. [warning] ofGstUtils: didn't received EOS in 5s, closing pipeline anyway [ error ] ofGstVideoUtils: buffer_cb(): error on new buffer, buffer size: 834632!= init size: 833340 [ error ] ofGstUtils: gstHandleMessage(): embedded video playback halted for plugin, module qtdemux1 reported: GStreamer encountered a general stream error.

This error does not appear if don't set the pixels to the default format and everything renders fine.

It also does not appear for the fingers.mov for example, even if I set the default format.

In addition it seems to me that this error is actually misleading, since if I force the player to render on the default format I will get a result that seems to me correct i.e the movie seems to render properly on the default colorspace .

EDIT: Not sure about the above comment anymore since I just tried the programmable renderer to see if the colorspace conversion would work properly but it doesn't.. The video is being rendered like this:

testvideo

I am using the legacy gl renderer and not the programmable one .

Here are some info of the video file that might be helpful:

General Complete name : test.mp4 Format : MPEG-4 Format profile : Base Media / Version 2 Codec ID : mp42 File size : 265 MiB Duration : 1mn 13s Overall bit rate : 30.1 Mbps Encoded date : UTC 2014-10-09 10:19:24 Tagged date : UTC 2014-10-09 10:19:28 ©TIM : 00;00;00;00 ©TSC : 2997 ©TSZ : 50

Video ID : 1 Format : AVC Format/Info : Advanced Video Codec Format profile : High@L5.1 Format settings, CABAC : Yes Format settings, ReFrames : 3 frames Format settings, GOP : M=3, N=33 Codec ID : avc1 Codec ID/Info : Advanced Video Coding Duration : 1mn 13s Source duration : 1mn 13s Bit rate : 30.1 Mbps Width : 860 pixels Height : 646 pixels Display aspect ratio : 4:3 Frame rate mode : Variable Frame rate : 59.940 fps Minimum frame rate : 59.940 fps Maximum frame rate : 60.000 fps Standard : NTSC Color space : YUV Chroma subsampling : 4:2:0 Bit depth : 8 bits Scan type : Progressive Bits/(Pixel*Frame) : 0.904 Stream size : 264 MiB (100%) Source stream size : 264 MiB (100%) Language : English Encoded date : UTC 2014-10-09 10:19:24 Tagged date : UTC 2014-10-09 10:19:24 Color primaries : BT.709 Transfer characteristics : BT.709 Matrix coefficients : BT.709 mdhd_Duration : 73724

arturoc commented 10 years ago

can you set the logging to verbose and post the output of the application? ofSetLogLevel(OF_LOG_VERBOSE)

PetrosKataras commented 10 years ago

My bad should have included already.. Here you go ( although it doesn't seem to report much more info regarding the actual problem.. )

[verbose] ofGstUtils: gstreamer inited [verbose] ofGstVideoPlayer: loadMovie(): loading "file:///home/petroska/Desktop/colorspaceOF/openFrameworks/apps/testColorSpace/bin/data/test.mp4" [verbose] ofGstUtils: startPipeline(): attaching callbacks [verbose] ofGstUtils: setSpeed(): speed changed to 1 [verbose] ofGstVideoPlayer: allocating as 860x646 raw video I420 [notice ] ofGstVideoPlayer: allocating with 860x646 I420 [verbose] ofShader: checkAndCreateProgram(): creating GLSL program [verbose] ofShader: setupShaderFromSource(): GL_VERTEX_SHADER shader compiled [verbose] ofShader: setupShaderFromSource(): GL_FRAGMENT_SHADER shader compiled [verbose] linkProgram(): attaching GL_FRAGMENT_SHADER shader to program 28 [verbose] linkProgram(): attaching GL_VERTEX_SHADER shader to program 28 [verbose] ofShader: checkProgramLinkStatus(): program 28linked [verbose] ofShader: checkAndCreateProgram(): creating GLSL program [verbose] ofShader: setupShaderFromSource(): GL_VERTEX_SHADER shader compiled [verbose] ofShader: setupShaderFromSource(): GL_FRAGMENT_SHADER shader compiled [verbose] linkProgram(): attaching GL_FRAGMENT_SHADER shader to program 31 [verbose] linkProgram(): attaching GL_VERTEX_SHADER shader to program 31 [verbose] ofShader: checkProgramLinkStatus(): program 31linked [ error ] ofGstVideoUtils: buffer_cb(): error on new buffer, buffer size: 834632!= init size: 833340 [ error ] ofGstUtils: gstHandleMessage(): embedded video playback halted for plugin, module qtdemux0 reported: GStreamer encountered a general stream error.

PetrosKataras commented 10 years ago

Looked a bit more into this today ..

The problem seems to be padding on the U and V planes.

I am checking the strides of the planes with vinfo.stride and for the Y plane I get 860 but for the U and V planes I get 432 for both of them instead of 430. This seems to be related to the divisible by 4 principle of gstreamer that was mentioned in the initial forum thread .

Is there any way to account for this padding ? I can see that the setFromAlignedPixels uses the 1st plane for the stride and I was wondering what would be the best way to deal with this right now..

arturoc commented 10 years ago

i think the fastest way to do this would be by having a padding or stride parameter in ofPixels so ofTexture coulñd take that into account and use the correct glPixelStorei to upload without having to make any copies in ram

like-a-bause commented 9 years ago

I got similar issues but somehow different and made some tests. My testsetup consist of archlinux with gstreamer 1.6 and latest of master. The same applies to Ubuntu 14.04 with gstreamer 1.2. My testApp consists of the ProgammableRenderer and an ofVideoPlayer with pixelFormat set to OF_PIXELS_NATIVE. My Test video is h264 e.g. i420. With a resolution of 1280x720 I get the correct video played: screenshot from 2015-10-19 21-05-14 With a resolution of 1278x720 and 1282x720 i get this: screenshot from 2015-10-19 21-05-22 I made process_sample a bit more verbose and became for the stock video:

[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: GstVideoInfo: 
[notice ] ofGstVideoUtils::process_sample: stride[0]: 1280
[notice ] ofGstVideoUtils::process_sample: stride[1]: 640
[notice ] ofGstVideoUtils::process_sample: stride[2]: 640
[notice ] ofGstVideoUtils::process_sample: width: 1280
[notice ] ofGstVideoUtils::process_sample: height: 720
[notice ] ofGstVideoUtils::process_sample: offset[0]: 0
[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: ofPixels: 
[notice ] ofGstVideoUtils::process_sample: pixels.width(): 1280
[notice ] ofGstVideoUtils::process_sample: pixels.height(): 720

and for the faulty displayed video (1278x720):

[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: GstVideoInfo: 
[notice ] ofGstVideoUtils::process_sample: stride[0]: 1280
[notice ] ofGstVideoUtils::process_sample: stride[1]: 640
[notice ] ofGstVideoUtils::process_sample: stride[2]: 640
[notice ] ofGstVideoUtils::process_sample: width: 1278
[notice ] ofGstVideoUtils::process_sample: height: 720
[notice ] ofGstVideoUtils::process_sample: offset[0]: 0
[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: ofPixels: 
[notice ] ofGstVideoUtils::process_sample: pixels.width(): 1278
[notice ] ofGstVideoUtils::process_sample: pixels.height(): 720

and (1282x720):

[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: GstVideoInfo: 
[notice ] ofGstVideoUtils::process_sample: stride[0]: 1284
[notice ] ofGstVideoUtils::process_sample: stride[1]: 644
[notice ] ofGstVideoUtils::process_sample: stride[2]: 644
[notice ] ofGstVideoUtils::process_sample: width: 1282
[notice ] ofGstVideoUtils::process_sample: height: 720
[notice ] ofGstVideoUtils::process_sample: offset[0]: 0
[notice ] ofGstVideoUtils::process_sample: --------------------
[notice ] ofGstVideoUtils::process_sample: ofPixels: 
[notice ] ofGstVideoUtils::process_sample: pixels.width(): 1282
[notice ] ofGstVideoUtils::process_sample: pixels.height(): 720

It seems that it doesn't matter if the stride of the y plane is a multiple of the u and v planes or not. In these cases https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/video/ofGstUtils.cpp#L1303 always gets taken. I had a look at https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/graphics/ofPixels.cpp#L315 and it seems to do the right thing. Cutting of the extra padding bytes. I have no idea how the planes are mapped in memory. Where does the upload to Texture happen? Pinging @arturoc In addition: When converting the same video with ffmpeg to 900x300 i get a green videoframe and run into (I'm not sure whether this also happens on ubuntu 14.04 with a previous of version)

[ error ] ofGstVideoUtils: buffer_cb(): error on new buffer, buffer size: 405600!= init size: 405000

To give credit for the testvideo: https://vimeo.com/131874019 brokchrd (loop) by beeple

arturoc commented 9 years ago

mmh, ok we probably should have some kind of padding attribute in ofPixels so we can add that in the texture when uploading the data otherwise we would need to make copies of all the planes and then stitch them together in the frame call back which would be pretty slow.

this is kind of complex so it won't get fixed for 0.9 but i'm milestoning it for next release and will take a look when 0.9 is done

like-a-bause commented 9 years ago

I need to quick-fix this now. I added another setFromAlignedPixels():

template<typename PixelType>
void ofPixels_<PixelType>::setFromAlignedPixels(const PixelType * newPixels, int width, int height, ofPixelFormat _pixelFormat, int stride_0, int stride_1) {
    int channels = channelsFromPixelFormat(_pixelFormat);
    if(channels==0) return;

    if(width*channels==stride_0){
        setFromPixels(newPixels,width,height,_pixelFormat);
        return;
    }

    allocate(width, height, _pixelFormat);

    const unsigned char* src = (unsigned char*) newPixels;
    unsigned char* dst =  (unsigned char*) pixels;
    // Y Plane
    for(int i = 0; i < height; i++) {
        memcpy(dst, src, width);
        src += stride_0;
        dst += width;
    }
    // U Plane
    for(int i = 0; i < height /2; i++){
        memcpy(dst,src,width/2);
        src += stride_1;
        dst += width/2;
    }
    // V Plane
    for(int i = 0; i < height /2; i++){
        memcpy(dst,src,width/2);
        src += stride_1;
        dst += width/2;
    }
}

and swapped it in here:

backPixels.setFromAlignedPixels(mapinfo.data,pixels.getWidth(),pixels.getHeight(),pixels.getPixelFormat(),stride, v_info.stride[1]);

Works for the two testvideos with awkward resolutions and i420. Haven't tested other video formats. It might be, that this broke some other cases.

arturoc commented 9 years ago

you could add something more generic like:

template<typename PixelType>
void ofPixels_<PixelType>::setFromAlignedPixels(const PixelType * newPixels, int width, int height, ofPixelFormat _pixelFormat, std::vector<int> stride) {

and pass a stride per plane, if you want to add this, even if it only works for I420 by now i'll merge it since it's the most common format and will solve this problem for a lot of cases then we can look for a better solution for next release.

for other formats you could add a warning

like-a-bause commented 9 years ago

yeah, right. vector of stride is much cleaner. i will assemble a pull request!

bilderbuchi commented 9 years ago

closed by #4456

like-a-bause commented 9 years ago

Wait a minute, wait a minute, wait a minute. This isn't done yet. I got another test video with 900x300. Which tells me:

[verbose] ofGstVideoUtils::process_sample: --------------------
[verbose] ofGstVideoUtils::process_sample: GstVideoInfo: 
[verbose] ofGstVideoUtils::process_sample: gstmapinfo.size: 405600
[verbose] ofGstVideoUtils::process_sample: gstmapinfo.maxsize: 405615
[verbose] ofGstVideoUtils::process_sample: stride[0]: 900
[verbose] ofGstVideoUtils::process_sample: stride[1]: 452
[verbose] ofGstVideoUtils::process_sample: stride[2]: 452
[verbose] ofGstVideoUtils::process_sample: width: 900
[verbose] ofGstVideoUtils::process_sample: height: 300
[verbose] ofGstVideoUtils::process_sample: offset[0]: 0
[verbose] ofGstVideoUtils::process_sample: offset[1]: 270000
[verbose] ofGstVideoUtils::process_sample: offset[2]: 337800
[verbose] ofGstVideoUtils::process_sample: --------------------
[verbose] ofGstVideoUtils::process_sample: ofPixels: 
[verbose] ofGstVideoUtils::process_sample: pixels.getTotalBytes(): 405000
[verbose] ofGstVideoUtils::process_sample: pixels.width(): 900
[verbose] ofGstVideoUtils::process_sample: pixels.height(): 300

So here stride[0] is as you would expect, but the strides of U anv V are padded. To get this going I needed to:

  1. uncomment this line

This is not applicable for i420 anymore:

if(stride == (pixels.getWidth() * pixels.getBytesPerPixel())) {

Anyway to come by I would add another format check and only do that if the format != i420. Other suggestions?

arturoc commented 9 years ago

i think there should be a general formula to calculate that but i haven't seen any video where the reported stride was wrong so we can probably just remove that check safely.

bilderbuchi commented 9 years ago

Wait a minute, wait a minute, wait a minute. This isn't done yet.

Well, if your PR doesn't fix this issue, then you shouldn't name it "fix for #3404" ;-)