when using mplayer to play back a mvc file, and see the garbage data output.
when using gstreamer to play back a mvc file. SIGSEGV arise. log:
[root@x-bdw08 mvc_enc_cfm]# gst-launch-1.0 filesrc location=/tmp/gstEncJMVCdump.264  ! vaapiparse_h264 ! vaapidecode ! vaapisink sync=false
libva info: VA-API version 0.35.2
libva info: va_getDriverName() returns 0
libva info: Trying to open /opt/X11R7/lib/dri/i965_drv_video.so
libva info: Found init function __vaDriverInit_0_35
libva info: va_openDriver() returns 0
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Got context from element 'vaapidecode0': gst.vaapi.Display=context, display=(GstVaapiDisplay)NULL;
Caught SIGSEGV

+ Trace 233728

Created attachment 279267 [details] [review]
enc:h264: Add missing field in packed subset sequence parameter set

This patch will fix the crash. Now the encoding seems to be working . But still some issue to decode all the frames. I am looking into this.

Review of attachment 279267 [details] [review]:

LGTM. Thanks. I will push that when the whole encoder: h264 series is complete.

Created attachment 279299 [details] [review]
mvc_enc: h264: Fix nal_unit_types in packed headers.

Submit prefix nal headers(nal_unit_type = 14) before every packed slice  headers(nal_unit_type = 1 or 5) only for the base view. Add nal_unit_header_mvc_extenion for each packed slice header in non base view with nal_unit_type equal to 20(coded slice mvc extension nal unit)

Review of attachment 279299 [details] [review]:

LGTM overall. There is a possible space optimization to make (in another patch later if you want): if you guarantee VOIdx = 0 && view_id = 0 for the whole sequence, then you don't need the Prefix NAL unit for the base view.

Review of attachment 279299 [details] [review]:

Anyway, this patch looks fine.

Review of attachment 279267 [details] [review]:

Pushed to git master branch. Thanks.

Review of attachment 279299 [details] [review]:

Pushed to git master branch. Thanks.

I think the case of codec_mvc_slice_nal (type=20) is not handled in the decoder side. Could you please check that?

(In reply to comment #9)
> I think the case of codec_mvc_slice_nal (type=20) is not handled in the decoder
> side. Could you please check that?

The decoder is calling decode_slice() for Coded slice extension. Are you using the built-in vaapiparse_h264 element?

after using last gstreamer vaapi git master commit(173f32d8e5cb40616e8eeb2f63a4c8a11c91c158) to test the MVC encoding,
mvc stream that encoded by gstreamer can play well with mplayer and gstreamer.

but JMVC still fails to decode it.

(In reply to comment #11)
> after using last gstreamer vaapi git master
> commit(173f32d8e5cb40616e8eeb2f63a4c8a11c91c158) to test the MVC encoding,
> mvc stream that encoded by gstreamer can play well with mplayer and gstreamer.
> 
> but JMVC still fails to decode it.

MPlayer would only decode the base view, i.e. the standard AVC stream.

(In reply to comment #11)
> after using last gstreamer vaapi git master
> commit(173f32d8e5cb40616e8eeb2f63a4c8a11c91c158) to test the MVC encoding,
> mvc stream that encoded by gstreamer can play well with mplayer and gstreamer.
> 
> but JMVC still fails to decode it.

Did you manage to decode all frames with gstremaer-vaapi? When i tried to decode a sample with 500 view frames, gstreamer vaapi got failed after 48 frames. 
Just wanted to know whether you manage to decode all the frames , or at least all the frames except last couple of frames.

(In reply to comment #13)
> (In reply to comment #11)
> > after using last gstreamer vaapi git master
> > commit(173f32d8e5cb40616e8eeb2f63a4c8a11c91c158) to test the MVC encoding,
> > mvc stream that encoded by gstreamer can play well with mplayer and gstreamer.
> > 
> > but JMVC still fails to decode it.
> 
> Did you manage to decode all frames with gstremaer-vaapi? When i tried to
> decode a sample with 500 view frames, gstreamer vaapi got failed after 48
> frames. 
> Just wanted to know whether you manage to decode all the frames , or at least
> all the frames except last couple of frames.

All the necessary patches are tracked through bug #731831. If you construct a pipeline yourself (gst-launch filesrc ! vaapiparse_h264 ! vaapidecode ! vaapisink), the parser would choose a format that have issues.

I personally only test programatically so that to exercise decodebin, which gets it right.

I was trying with gst-launch-1.0 filesrc location=mvc_otc_master.264 ! vaapiparse_h264 ! "video/x-h264,stream-format=byte-stream,alignment=nal" ! vaapidecode ! vaapisink . 

Anyway the decoder is working fine for other reference streams .So it should be an issue with the encoder. What i need is a test from some one to make sure that the weird behavior which i am getting is consistent for all :)
Because the comment from zhixinx.liu is confusing me ,
"mvc stream that encoded by gstreamer can play well with mplayer and gstreamer."

(In reply to comment #15)
> I was trying with gst-launch-1.0 filesrc location=mvc_otc_master.264 !
> vaapiparse_h264 ! "video/x-h264,stream-format=byte-stream,alignment=nal" !
> vaapidecode ! vaapisink . 
> 
> Anyway the decoder is working fine for other reference streams .So it should be
> an issue with the encoder. What i need is a test from some one to make sure
> that the weird behavior which i am getting is consistent for all :)
> Because the comment from zhixinx.liu is confusing me ,
> "mvc stream that encoded by gstreamer can play well with mplayer and
> gstreamer."

The comment for gstreamer also confused me. If it does, but not with JMVC, a possible cause could be that the generated SPS headers are wrong, and MVC profiles could not be clearly identified, thus defaulting to AVC stream decoding?

Note: I don't know if this is the case right now, but all the required SPS headers should be within the same access unit, and preferably all before the primary coded slice.

Let me to clarify what the mean of "mvc stream that encoded by gstreamer can play well with mplayer and gstreamer."
1, using gstreamer1.2 to do the mvc encoder test
2, got the mvc encoded bit stream file from step 1
3, used mplayer and gstreamer1.2 to decode the bit stream file, they worked well.
4, used the jmvc to decode the bit stream file, it failed.

(In reply to comment #17)
> Note: I don't know if this is the case right now, but all the required SPS
> headers should be within the same access unit, and preferably all before the
> primary coded slice.

We are periodically sending sps and subset_sps from encoder during before each idr and which is causing issues for the decoder. What ever reference streams i have tested has only one sps and one subset sps. 

Have you ever tested the decoder with a sample which is having 2 or more sps headers?

Created attachment 279343 [details] [review]
Add cpbBrNalFactor values for Stereo/MVC profiles

Created attachment 279344 [details] [review]
encoder:h264: Fix the value for timing_info_present_flag in vui_parameters.

Created attachment 279345 [details] [review]
MVC_encode: h264: set number of anchor and non anchor refpic list to zero

Created attachment 279346 [details] [review]
Hack: To test the JMVC decoder: set vui_parameters_preset_flag to FALSE

This is a hack patch to make JMVC working. But still segfaulting after decoding a bunch of frames.  
Thanks to Xiang Haiho for this information :)

Added this patch only for testing since there is no reason to set vui_parameters_flag to zero for MVC as per spec.

HI Gwenole,

The gst-mvc-decoder is failing during second idr_pic decoding (if the decoder receives a new sps/subset_sps before each idr_pic) . Because the entire context is getting destroyed during ensure_context() invocation in gstvaapidecoder_h264.c. The max_view field is resetting to 1 during each sps parsing and the profile_idc is changing between base view and non base view which will cause the destruction of vacontext and the decoding is failing because of this.

Could you please have a look in to  this?
I think it needs to be fixed in gst-decoder. 

The JM_reference decoder is working fine for the generated stream. But the JMVC is still failing after sometime.
But if JMVC has some restriction like "always set vui_parameters_flag to zero" then we can't consider it as a ref decoder . isn't it :)

Anyway I will try to look into this a bit more.

as it's pv release feature. change the severity from normal to critical.

(In reply to comment #24)
> HI Gwenole,
> 
> The gst-mvc-decoder is failing during second idr_pic decoding (if the decoder
> receives a new sps/subset_sps before each idr_pic) . Because the entire context
> is getting destroyed during ensure_context() invocation in
> gstvaapidecoder_h264.c. The max_view field is resetting to 1 during each sps
> parsing and the profile_idc is changing between base view and non base view
> which will cause the destruction of vacontext and the decoding is failing
> because of this.
> 
> Could you please have a look in to  this?
> I think it needs to be fixed in gst-decoder. 

Probably move up the priv->max_views = 1; to the open() function? I think eoseq conditions could be tracked explicitly now + add priv->max_views = 1 into the handler for sequence_end.

The reason why max views was reset was because, conceptually, we could have MVC stream, then SVC stream, then AVC stream, etc. Though, I would believe an EOSEQ to be required in that case.

> The JM_reference decoder is working fine for the generated stream. But the JMVC
> is still failing after sometime.
> But if JMVC has some restriction like "always set vui_parameters_flag to zero"
> then we can't consider it as a ref decoder . isn't it :)

That's probably a leftover because they commented the check for subset sps equivalent. Please comment it in the JMVC sps part too.

Review of attachment 279343 [details] [review]:

OK

Review of attachment 279344 [details] [review]:

You need to also mention or ensure that vui_parameters_present_flag == 1 + add comment in source too (H.7.4.2.1.1). I need to track what portions are implemented, and I would like to avoid digging into the git commit logs. Thanks. :)

Review of attachment 279345 [details] [review]:

LGTM. Even though it would probably be cleaner to only keep WRITE_UE(0) of the count numbers, and drop the loops?

(In reply to comment #26)
> > The JM_reference decoder is working fine for the generated stream. But the JMVC
> > is still failing after sometime.
> > But if JMVC has some restriction like "always set vui_parameters_flag to zero"
> > then we can't consider it as a ref decoder . isn't it :)
> 
> That's probably a leftover because they commented the check for subset sps
> equivalent. Please comment it in the JMVC sps part too.

Well, this means, implement vui_parameters() there too. :)
Or keep vui_parameters_present_flag to FALSE and try to check why it fails?

Created attachment 279366 [details] [review]
 encoder:h264: Fix the value for timing_info_present_flag  in vui_parameters.

(In reply to comment #29)
> Review of attachment 279345 [details] [review]:
> 
> LGTM. Even though it would probably be cleaner to only keep WRITE_UE(0) of the
> count numbers, and drop the loops?

I prefer to keep the code as it is for more clarity.

I know why the jmvc is failing :)
The JMVC decoder is failing if it found a second sps with in the stream.!!

An eg: case:
Make an mvc stream which has only one sps & subsetsps by setting the keyframe-period==number of frames.

 gst-launch-1.0 -v videotestsrc num-buffers=250 ! "video/x-raw, format=(string)NV12, width=640, height=480" ! vaapiencode_h264 num-views=2 keyframe-period=250  !  filesink location=mvc.264

Now the JMVCdecoder can decode all the frames !

So the issue should be because of incomplete JMVC-decoder implementation.
What do you think?

Review of attachment 279343 [details] [review]:

Pushed to git master branch. Thanks.

Review of attachment 279345 [details] [review]:

Pushed to git master branch. Thanks.

Review of attachment 279366 [details] [review]:

Pushed to git master branch. Thanks.

(In reply to comment #33)
> I know why the jmvc is failing :)
> The JMVC decoder is failing if it found a second sps with in the stream.!!
> 
> An eg: case:
> Make an mvc stream which has only one sps & subsetsps by setting the
> keyframe-period==number of frames.
> 
>  gst-launch-1.0 -v videotestsrc num-buffers=250 ! "video/x-raw,
> format=(string)NV12, width=640, height=480" ! vaapiencode_h264 num-views=2
> keyframe-period=250  !  filesink location=mvc.264
> 
> Now the JMVCdecoder can decode all the frames !
> 
> So the issue should be because of incomplete JMVC-decoder implementation.
> What do you think?

Well, in practice, we don't really need to submit SPS/PPS every I-frame. A better bet would be every IDR frame, but still. I believe, in general, we should probably just submit a single SPS, and let upper elements manage recovery/copy/propagation at fixed intervals, maybe.

Created attachment 279401 [details] [review]
encoder: h264: generate only one sps/subset_sps at the beginning of a stream.

This patch is for generating the sps/subset_sps only once for the stream. This will make jmvc happy . But the generated stream has issues with gstreamer-vaapi decoder.

options to proceed:
apply this patch: fix gst-decoder (since jmvc and jm are able to decode the generated stream)
don't apply this patch: fix gst-decoder (since it needs to handle the second sps)

(In reply to comment #38)
> Created an attachment (id=279401) [details] [review]
> encoder: h264: generate only one sps/subset_sps at the beginning of a stream.
> 
> This patch is for generating the sps/subset_sps only once for the stream. This
> will make jmvc happy . But the generated stream has issues with gstreamer-vaapi
> decoder.
> 
> options to proceed:
> apply this patch: fix gst-decoder (since jmvc and jm are able to decode the
> generated stream)

this should read "dont-fix gst-decoder"?

> don't apply this patch: fix gst-decoder (since it needs to handle the second
> sps)

Hmmm, on the other hand, if we receive an SPS/subset SPS header on the decoder side, this should not affect the process since an IDR slice would follow. i.e. a reset of the underlying VA context should have zero impact IMHO.

(In reply to comment #39)
> (In reply to comment #38)

> > apply this patch: fix gst-decoder (since jmvc and jm are able to decode the
> > generated stream)
> 
> this should read "dont-fix gst-decoder"?
> 
> > don't apply this patch: fix gst-decoder (since it needs to handle the second
> > sps)


Hmm, sorry my sentence was misleading.

With or with out this patch, the gst-decoder needs to be fixed i think.

If we didn't apply this patch the decoder needs a fix to handle the multiple sps.

If we apply this patch, the decoder still need some fix because it is failing to decode the generated stream.  But JMVC and JM are able to decode the corresponding stream.

Review of attachment 279401 [details] [review]:

Pushed a variant of the patch that generates SPS headers only if the codec config (resolution, bitrate, etc.) actually changed.

(In reply to comment #41)
> (In reply to comment #39)
> > (In reply to comment #38)
> 
> > > apply this patch: fix gst-decoder (since jmvc and jm are able to decode the
> > > generated stream)
> > 
> > this should read "dont-fix gst-decoder"?
> > 
> > > don't apply this patch: fix gst-decoder (since it needs to handle the second
> > > sps)
> 
> 
> Hmm, sorry my sentence was misleading.
> 
> With or with out this patch, the gst-decoder needs to be fixed i think.
> 
> If we didn't apply this patch the decoder needs a fix to handle the multiple
> sps.
> 
> If we apply this patch, the decoder still need some fix because it is failing
> to decode the generated stream.  But JMVC and JM are able to decode the
> corresponding stream.

Opted for the second option, i.e. apply the proposed patch (variant). Besides, your decoding issue with gstreamer-vaapi should also be fixed. Please try it out. Thanks.

Multiple sps is still failing to decode. But the current master (with single sps for entire stream) is working with out any problem. Thanks for the patch !

So now the only problem left is with the JMVC_Decoder, and of course that needs to be fixed inside JMVC.

Created attachment 279456 [details] [review]
Patch to JMVC

This will fix the issue in JMVC decoder. Please have a test.

(In reply to comment #44)
> Multiple sps is still failing to decode. But the current master (with single
> sps for entire stream) is working with out any problem. Thanks for the patch !

That should be fixed now.

Completed the MVC encoding test, The total case number is 20, there are 5 cases failed(Segmentation fault when using JMVC to decode the MVC steam that encoding by gsteamer).
case name:
File name          width        height       view point
MVCNV-1.264          640            480            3
MVCNV-2.264          640            480            8
MVCNV-3.264          640            480            3
MVCNV-4.264          640            480            4
MVCSPS-2.264         640            480            8

(gdb) bt

+ Trace 233744

as most of test cases passed. change the severity from critical to normal

Support for more than two views will be tracked in bug #732453. Please focus on "num-views" = 2 for this bug, and close it if yours tests with 2 views work. Thanks.

close it as mvc encoding works with 2 views