After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 768647 - [MVC][Encode] all the encoded file can't be displayed.
[MVC][Encode] all the encoded file can't be displayed.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-07-11 07:17 UTC by Fei
Modified: 2016-07-29 06:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
encoder: h264: Fix MVC encode while enabling dct8x8 (1.22 KB, patch)
2016-07-12 12:02 UTC, sreerenj
none Details | Review
encoder: h264: Fix MVC encode while enabling dct8x8 (1.23 KB, patch)
2016-07-12 12:28 UTC, sreerenj
none Details | Review

Description Fei 2016-07-11 07:17:59 UTC
1. Testing Steps:
========================================================================
gst-launch-1.0 -v filesrc location=/media/JMVCdump.yuv '!' videoparse format=i420 width=640 height=480 '!' vaapih264enc num-views=2 keyframe-period=30 '!' filesink location=/tmp/gstEncJMVCdump.264

mplayer /tmp/gstEncJMVCdump.264

Then the pictures displayed is gray and unclear.

2. Testing Env:
========================================================================
gst_plugins_vaapi:   (master)6970dc127764129a6bcb06fcceb600e057346796
libva:               (master)c36971c682d890681fe839bbaa8a348fe845aa42
Libva_intel_driver:  (master)bcde10dac40cbc4c8502fa519404c9379372184b

3. Frequency of Occurence:
========================================================================
100%

4. Regression info:
========================================================================
gst-vaapi:fec82052d54daf1338f30d655a12fe6a72ea0a7f is OK.
Comment 1 sreerenj 2016-07-12 09:23:25 UTC
Thanks for the bug report.

This is the regression introduced by 
commit 4aec5bdd7207fc0e45813ef14c9c0ad5174a8f75
Author: Scott D Phillips <scott.d.phillips@intel.com>
Date:   Wed Jun 22 14:28:44 2016 -0700

    encoder: h264: Use high profile by default
    
    Change defaults for max-bframes, cabac, and dct8x8 to be enabled
    by default. This will cause the default profile to be high instead
    of baseline. In most situations this is the right decision, and
    the profile can still be lowered in the case of caps restrictions.
    
    Signed-off-by: Scott D Phillips <scott.d.phillips@intel.com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757941


If you manually set "dct8x8=false" the issue will get fixed, but of course that is not the solution we need.

Probably a diver fix needed.
Comment 2 Víctor Manuel Jáquez Leal 2016-07-12 10:22:45 UTC
@sree, shall we revert the commit?
Comment 3 sreerenj 2016-07-12 11:01:16 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #2)
> @sree, shall we revert the commit?

Yup, make sense..please do
we can get back to this once we address both of the issues correctly:
issue1: dct8x8=true giving garbage in encoded video
issue2: tune=high_compression as default, 
https://bugzilla.gnome.org/show_bug.cgi?id=757941#c5
Comment 4 sreerenj 2016-07-12 11:22:00 UTC
(In reply to sreerenj from comment #3)
> (In reply to Víctor Manuel Jáquez Leal from comment #2)
> > @sree, shall we revert the commit?
> 
> Yup, make sense..please do
> we can get back to this once we address both of the issues correctly:
> issue1: dct8x8=true giving garbage in encoded video
> issue2: tune=high_compression as default, 
> https://bugzilla.gnome.org/show_bug.cgi?id=757941#c5

Sorry, please wait, i will provide a proper fix...
Comment 5 sreerenj 2016-07-12 12:02:34 UTC
Created attachment 331318 [details] [review]
encoder: h264: Fix MVC encode while enabling dct8x8

encoder: h264: Fix MVC encode while enabling dct8x8
    
Pack the transform_8x8_mode_flag and other necessary rbsp data
in packed_pps header for MVC encode.
    
https://bugzilla.gnome.org/show_bug.cgi?id=768647
Comment 6 sreerenj 2016-07-12 12:03:52 UTC
@Fei, Please test the attached patch.
Comment 7 sreerenj 2016-07-12 12:28:42 UTC
Created attachment 331322 [details] [review]
encoder: h264: Fix MVC encode while enabling dct8x8

Attached a wrong patch before, please use this one for testing..
Comment 8 Fei 2016-07-13 06:24:22 UTC
(In reply to sreerenj from comment #6)
> @Fei, Please test the attached patch.

With this patch, the encoded files can be played by mplayer, but our test result still fail 15 case(total 20), I am looking for more details...
Comment 9 Fei 2016-07-13 07:29:44 UTC
(In reply to Fei from comment #8)
> (In reply to sreerenj from comment #6)
> > @Fei, Please test the attached patch.
> 
> With this patch, the encoded files can be played by mplayer, but our test
> result still fail 15 case(total 20), I am looking for more details...

This fail because of lose frame.

steps:
1. gst-launch-1.0 -v filesrc location=/media/a.yuv '!' videoparse format=i420 width=640 height=480 '!' vaapih264enc num-views=2 keyframe-period=30 '!' filesink location=/tmp/gstEncJMVCdump.264
2. $tools_path/jmvc8.5/bin/H264AVCDecoderLibTestStatic "/tmp/gstEncJMVCdump.264" "b.yuv" "2"
3 ls -al

Then a.yuv size is different with b.yuv .
Comment 10 sreerenj 2016-07-13 07:38:32 UTC
(In reply to Fei from comment #9)
> (In reply to Fei from comment #8)
> > (In reply to sreerenj from comment #6)
> > > @Fei, Please test the attached patch.
> > 
> > With this patch, the encoded files can be played by mplayer, but our test
> > result still fail 15 case(total 20), I am looking for more details...
> 
> This fail because of lose frame.
> 
> steps:
> 1. gst-launch-1.0 -v filesrc location=/media/a.yuv '!' videoparse
> format=i420 width=640 height=480 '!' vaapih264enc num-views=2
> keyframe-period=30 '!' filesink location=/tmp/gstEncJMVCdump.264
> 2. $tools_path/jmvc8.5/bin/H264AVCDecoderLibTestStatic
> "/tmp/gstEncJMVCdump.264" "b.yuv" "2"
> 3 ls -al
> 
> Then a.yuv size is different with b.yuv .

Was that working before?? For eg, with "fec82052d54daf1338f30d655a12fe6a72ea0a7f"  (since you mentioned it as the head which was working with out regression)
Comment 11 Fei 2016-07-13 07:45:26 UTC
yes, fec82052d54daf1338f30d655a12fe6a72ea0a7f works.
Comment 12 sreerenj 2016-07-13 08:28:51 UTC
(In reply to Fei from comment #11)
> yes, fec82052d54daf1338f30d655a12fe6a72ea0a7f works.

How did you find the "lose of frames" ?

Size difference is not because of "lose of frames",
Previously by default we were not using B-frames, and other coding tools were also different. Now the default pipeline uses B-frames, cabac=true dct8x8 =true by default...So the output will not be same as before which is expected..
Comment 13 Fei 2016-07-14 05:58:04 UTC
I removed "lose frames" judgement from our test system, and get 3 fail case:
1./media/mvc/MVCICT-1.264
2./media/mvc/MVCICT-2.264
3./media/mvc/MVCSPS-2.264

With "fec82052d54daf1338f30d655a12fe6a72ea0a7f", only item 1 and item 2 fail. Item 3 pass.

steps:
1. $tools_path/jmvc8.5/bin/H264AVCDecoderLibTestStatic /media/mvc/MVCDS-1.264 a.yuv 2

2. gst-launch-1.0 -v filesrc location=/media/a.yuv '!' videoparse format=i420 width=640 height=480 '!' vaapih264enc num-views=2 keyframe-period=30 '!' filesink location=/tmp/gstEncJMVCdump.264

3. $tools_path/jmvc8.5/bin/H264AVCDecoderLibTestStatic "/tmp/gstEncJMVCdump.264" "b.yuv" "2"

4. YSNR=$tools_path/metrics_calc_lite.ia32e -i1 a.yuv -i2 b.yuv -w 640 -h 480 -nopfm psnr y
   USNR=$tools_path/metrics_calc_lite.ia32e -i1 a.yuv -i2 b.yuv -w 640 -h 480 -nopfm psnr u
   VSNR=$tools_path/metrics_calc_lite.ia32e -i1 a.yuv -i2 b.yuv -w 640 -h 480 -nopfm psnr v

5. Only YSNR&USNR&VSNR all more than 40.0, then it pass, else it fail.

6. item 1 & item 2 YSNR is about 39.0+, Item 3 YSNR is 27.0+ with this patch.
Comment 14 sreerenj 2016-07-14 09:50:18 UTC
(In reply to Fei from comment #13)
> I removed "lose frames" judgement from our test system, and get 3 fail case:
> 1./media/mvc/MVCICT-1.264
> 2./media/mvc/MVCICT-2.264
> 3./media/mvc/MVCSPS-2.264
> 
> With "fec82052d54daf1338f30d655a12fe6a72ea0a7f", only item 1 and item 2
> fail. Item 3 pass.
> 
> steps:
> 1. $tools_path/jmvc8.5/bin/H264AVCDecoderLibTestStatic
> /media/mvc/MVCDS-1.264 a.yuv 2
> 
> 2. gst-launch-1.0 -v filesrc location=/media/a.yuv '!' videoparse
> format=i420 width=640 height=480 '!' vaapih264enc num-views=2
> keyframe-period=30 '!' filesink location=/tmp/gstEncJMVCdump.264
> 
> 3. $tools_path/jmvc8.5/bin/H264AVCDecoderLibTestStatic
> "/tmp/gstEncJMVCdump.264" "b.yuv" "2"
> 
> 4. YSNR=$tools_path/metrics_calc_lite.ia32e -i1 a.yuv -i2 b.yuv -w 640 -h
> 480 -nopfm psnr y
>    USNR=$tools_path/metrics_calc_lite.ia32e -i1 a.yuv -i2 b.yuv -w 640 -h
> 480 -nopfm psnr u
>    VSNR=$tools_path/metrics_calc_lite.ia32e -i1 a.yuv -i2 b.yuv -w 640 -h
> 480 -nopfm psnr v
> 
> 5. Only YSNR&USNR&VSNR all more than 40.0, then it pass, else it fail.
> 
> 6. item 1 & item 2 YSNR is about 39.0+, Item 3 YSNR is 27.0+ with this patch.


I don't know how accurate your psnr calculations here :)
For me , The psnr calculations with ffmpeg showing
"PSNR y:41.24 u:45.97 v:46.55 average:42.33 min:41.56 max:42.56"


Few things:
-- try setting dct8x8=FALSE while doing encoding, then set dct8x8=FALSE, cabac=FALSE in another test case. Not only for mvc, you can test with other normal streams too. compare the values and if there are many mismatches, you should contact driver team.

--For me, H264AVCDecoderLibTestStatic giving two views separately as two yuv files.
Comment 15 Fei 2016-07-15 10:40:02 UTC
If PSNR from ffmpeg is more than 40.0, then we can close this bug, and update our test script.
Comment 16 sreerenj 2016-07-15 11:37:58 UTC
Please don't close the bug until the patch reach in master branch :)

Going to push the patch...
Comment 17 sreerenj 2016-07-15 11:50:48 UTC
commit 6c6007ad94c402d33da9c5c73620e84064ea8fb2
Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com>
Date:   Fri Jul 15 14:41:27 2016 +0300

    encoder: h264: Fix MVC encode while enabling dct8x8
    
    Pack the transform_8x8_mode_flag and other necessary rbsp data
    in packed_pps header for MVC encode.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=768647
Comment 18 Fei 2016-07-18 02:13:42 UTC
(In reply to sreerenj from comment #16)
> Please don't close the bug until the patch reach in master branch :)
> 
> Going to push the patch...

That right:), mark this bug as reopen, and I will check this issue by using master branch in my next bi-weekly test.
Comment 19 Fei 2016-07-29 06:23:04 UTC
verified OK:
libva:
commit f7e22630bd18ed80d22da20a9dc18487938b6c8a
libva-intel:
commit 9fc4b6675c42df9002279fbc85985dcdd7510525
gst-vaapi:
commit ca0c3fd627e20eb261aaa79eff7a69ef028e467e