Add: [droid] hw acceleration via libstagefright #1832

Closed
wants to merge 18 commits into
from

Projects

None yet

9 participants

@koying
Member
koying commented Nov 22, 2012

This PR to monitor progress on providing accelerated android decoding via libstagefright

@Montellese
Member

OK I gave this a try on my SGS2 and apart from the mentioned issue it compiled fine. I was able to play normal AVI files as I used to and then I tried a 1080p mp4 which I recorded with my phones camera. The result was a very green-blueish image and it didn't seem accelerated to me (i.e. very jerky). This is the part of the log where I start playing the video: http://xbmclogs.com/show.php?id=15380. So it looks like it uses libstagefright.
Afterwards I wasn't able to close XBMC anymore (only force close through android's task manager) and after a restart of xbmc I couldn't play any h264 files anymore. It just kept on going in the busy dialog and there was nothing in logcat.

@koying
Member
koying commented Nov 24, 2012

IIRC, the blue-greenish ugly image is what you get when libstagefright (the sys so) is software rendering...

@koying
Member
koying commented Nov 24, 2012

Note that will checking other players (e.g. vplayer, where hw decoding can be switched), I came to realize most are actually able to render my test file (a 720p) in software flawlessly, so there might be more than HW decoding at stake...

@koying
Member
koying commented Dec 7, 2012

Moved the libstagefright stuff out of ffmpeg and inside dvdplayer as an hardware codec. That allows proper fallback to ffmpeg software decoding if a stagefright hw codec is not available.

Working great in 720p on my nexus 7

@Memphiz
Member
Memphiz commented Dec 7, 2012

Nice work! Sry if my comments came twice. Fuckin IE8 here ;)

I really like the fact of having a stagefright codec. Also putting the needed sources into the deps is a nice idea imo.

3 questions out of curiousity:

  1. Does it handle 1080p aswell?
  2. Does it work different/better as the internal ffmpeg impl?
  3. You tried anything else beside h.264? (seems a part of the code is already prepared for mpeg2, mpeg4part2 and vc1)?
@davilla
davilla commented Dec 7, 2012

nice

@koying
Member
koying commented Dec 7, 2012

@Memphiz

  1. Not yet properly. I suspect scaling issues. Working on it...
  2. This one works seamlessly while the ffmpeg one didn't. Plus I can now specify that I want hw codecs only and fallback to ffmpeg otherwise
  3. Not yet. 1080p h264 is my primary focus for now...
@koying
Member
koying commented Dec 7, 2012

Note that this is ICS+ only. If we want to support both 2.3 and 4.0 with the same package, we'll have to go the DllImpl path...

@Memphiz
Member
Memphiz commented Dec 7, 2012

I would say we should focus on ICS and let gingerbread die in fire - but I guess TheUni would come up with better arguments then my stomac haha.

@koying
Member
koying commented Dec 7, 2012

Yeah. Anyway, I somewhat doubt a device still having GB is a good candidate to run XBMC...

@da-anda
Member
da-anda commented Dec 7, 2012

XBMC runs pretty well on my GB phone, and it's capable of playing 720p (tested with VLC I think) - so IMO we should support it if it doesn't add to much overhead

@Memphiz
Member
Memphiz commented Dec 8, 2012

I tried this branch and it just gives me a black screen (tested a 720p mkv h.264) - audio is working without issues - but black screen. Overlay shows stf-h264 usage.

http://pastebin.com/UCycLzdJ

In the log i also tried it with software decoding (which worked - but cpu load is to high for making it smooth).

This was tested on odroid-x running ICS 4.0.4 (kernel 3.0.41)

@koying
Member
koying commented Dec 8, 2012

Ok. I'll first focus on hw I have, i.e. tegra3 Nexus 7. If I get expected results, i.e. smooth h264 1080p, we'll see how we can build from there to support more devices...

@elupus elupus commented on an outdated diff Dec 8, 2012
.../cores/dvdplayer/DVDCodecs/Video/StageFrightVideo.cpp
+ if (status != OK)
+ {
+ CLog::Log(LOGERROR, "%s::%s - %s\n", CLASSNAME, __func__,"Error getting picture from frame");
+ return false;
+ }
+
+ pDvdVideoPicture->format = RENDER_FMT_YUV420P;
+ pDvdVideoPicture->pts = DVD_NOPTS_VALUE;
+ pDvdVideoPicture->dts = DVD_NOPTS_VALUE;
+ pDvdVideoPicture->color_range = 0;
+ pDvdVideoPicture->color_matrix = 4;
+ pDvdVideoPicture->iFlags = DVP_FLAG_ALLOCATED;
+ pDvdVideoPicture->iWidth = frame->width;
+ pDvdVideoPicture->iHeight = frame->height;
+ pDvdVideoPicture->iDisplayWidth = frame->width;
+ pDvdVideoPicture->iDisplayHeight = frame->height;
@elupus
elupus Dec 8, 2012 Team Kodi member

this is very likely not right. this should reflect aspect of frame

@davilla
davilla commented Jan 6, 2013

see https://github.com/Pivosgroup/xbmc, for new --enable-codec configure switch. I'd like to move any alternate codecs to use this switch. It can handle multiple items too. So something like

--enable-codec=amcodec,stfcodec

would enable the building of both amcodec and stfcodec

@koying
Member
koying commented Jan 6, 2013

ack

@davilla
davilla commented Jan 6, 2013

what is your current dev branch ? I want to start resolving some issues.

@koying
Member
koying commented Jan 6, 2013

Please hold on. I'm currently trying to avoid buffer copying thru textures.
I'll remove the [WIP] when the "final" PR will be ready for actual review/patching.

@koying
Member
koying commented Jan 13, 2013

Quite happy with the current state, so opened for review / patches. Still an issue with FF/Rewind.
ICS+ only.

I'll put up test builds in the forum as soon as they're built.

@davilla
davilla commented Jan 13, 2013

nice, look forward to testing

@davilla
davilla commented Jan 13, 2013

oh, --enable-codec=amcodec,stfcodec ?

@koying
Member
koying commented Jan 13, 2013

Re --enable-codec, did you already commit yours? I planned to rebase upon it..

@davilla
davilla commented Jan 13, 2013

k

@koying
Member
koying commented Jan 13, 2013

Updated for build issues

@davilla
davilla commented Jan 13, 2013

m_keyguardLock is not defined

@koying
Member
koying commented Jan 13, 2013

Has been removed in latest push, hasn't it?

@davilla
davilla commented Jan 13, 2013

yes, my bad, I'm a little behind.

Droid-U2, this worked before, now does not.

V/XBMC ( 2309): 19:54:17 T:1556104592 DEBUG: CDVDPlayer::HandleMessages - player started 2
V/XBMC ( 2309): 19:54:18 T:1556105112 DEBUG: CDVDPlayerAudio:: Discontinuity1 - was:554500.791667, should be:428009.082667, error:-126491.709000
V/XBMC ( 2309): 19:54:18 T:1556105112 DEBUG: CDVDPlayerAudio:: Discontinuity1 - was:761432.166667, should be:864000.000000, error:102567.833333
V/XBMC ( 2309): 19:54:18 T:1556105112 DEBUG: CDVDPlayerAudio:: Discontinuity1 - was:1267193.250000, should be:1396245.624667, error:129052.374667
E/OMXCodec( 2309): [OMX.SEC.AVC.Decoder] Timed out waiting for output buffers: 0/6
V/XBMC ( 2309): 19:54:20 T:1557301896 ERROR: CStageFrightVideo - decoding error (-110)
V/XBMC ( 2309): 19:54:20 T:1557301896 DEBUG: CDVDPlayerVideo - video decoder returned error
E/OMXCodec( 2309): [OMX.SEC.AVC.Decoder] Timed out waiting for output buffers: 0/6
V/XBMC ( 2309): 19:54:23 T:1557301896 ERROR: CStageFrightVideo - decoding error (-110)
V/XBMC ( 2309): 19:54:23 T:1557301896 DEBUG: CDVDPlayerVideo - video decoder returned error
E/OMXCodec( 2309): [OMX.SEC.AVC.Decoder] Timed out waiting for output buffers: 0/6
V/XBMC ( 2309): 19:54:26 T:1557301896 ERROR: CStageFrightVideo - decoding error (-110)
V/XBMC ( 2309): 19:54:26 T:1557301896 DEBUG: CDVDPlayerVideo - video decoder returned error
V/XBMC ( 2309): 19:54:27 T:1556105112 WARNING: CDVDMessageQueue(audio)::Get - asked for new data packet, with nothing available
V/XBMC ( 2309): 19:54:27 T:1556105112 DEBUG: CSoftAEStream::Flush
E/OMXCodec( 2309): [OMX.SEC.AVC.Decoder] Timed out waiting for output buffers: 0/6

@koying
Member
koying commented Jan 13, 2013

Fragmentation, fragmentation :-(

Could you share the full log, please. There might be messages earlier indicating the root cause..

@davilla
davilla commented Jan 13, 2013

http://pastebin.com/dS1i8EbL using your fulldebug image.

@koying
Member
koying commented Jan 13, 2013

Right. I think I remember this codec (and probably others) only allocate 2 output buffers, and both are locked by the renderer...

@davilla
davilla commented Jan 13, 2013

sounds about right.

@davilla
davilla commented Jan 17, 2013

time to update ?

@koying
Member
koying commented Jan 18, 2013

Coming... I think I found the best compromise: FBO's

@davilla
davilla commented Jan 19, 2013

davilla@rootcoder:/xbmc/xbmc-android$ file xbmc/cores/dvdplayer/DVDCodecs/Video/StageFrightVideo.cpp
xbmc/cores/dvdplayer/DVDCodecs/Video/StageFrightVideo.cpp: ASCII C program text, with CRLF line terminators

opps :)

@elupus elupus and 1 other commented on an outdated diff Feb 4, 2013
.../cores/dvdplayer/DVDCodecs/Video/StageFrightVideo.cpp
+ //frame->pts = (dts != DVD_NOPTS_VALUE) ? pts_dtoi(dts) : ((pts != DVD_NOPTS_VALUE) ? pts_dtoi(pts) : 0);
+ frame->pts = (pts != DVD_NOPTS_VALUE) ? pts_dtoi(pts) : ((dts != DVD_NOPTS_VALUE) ? pts_dtoi(dts) : 0);
+ frame->duration = 0;
+ frame->medbuf = p->getBuffer(demuxer_bytes);
+ if (!frame->medbuf)
+ {
+ free(frame);
+ return VC_ERROR;
+ }
+ fast_memcpy(frame->medbuf->data(), demuxer_content, demuxer_bytes);
+ frame->medbuf->meta_data()->clear();
+ frame->medbuf->meta_data()->setInt64(kKeyTime, frame->pts);
+
+ if (p->mPrevPts >= 0)
+ {
+ frame->duration = frame->pts - p->mPrevPts;
@elupus
elupus Feb 4, 2013 Team Kodi member

This will most often be wrong. Note that pts is not in decode order. B frames will (and must come first). Don't try to set duration from anything but the demuxer packet.

@koying
koying Feb 5, 2013 Team Kodi member

You mean from the decoded packet? We don't get the duration in ::Decode...

@elupus
elupus Feb 5, 2013 Team Kodi member

Right. Since normally you don't need it. If you absolutely need it we
should extend api, but I would need to know why you need it. Diffing pts is
wrong. You can diff dts if available, but I see no point.

@koying
koying Feb 5, 2013 Team Kodi member

The only point is to feed ::GetTimeSize. But I might get the concept wrong...

@elupus
elupus Feb 5, 2013 Team Kodi member

Just ignore gettimesize for now. It's not very important. You should avoid
buffering more than a single frame/packet anyway. Preferably you should not
buffer any, but that likely doesn't work for hw.

@koying
koying Feb 5, 2013 Team Kodi member

k. BTW, being a newbie in codec stuff, when should I use incoming dts vs.
pts as the frame tm?

On 5 February 2013 12:30, Joakim Plate notifications@github.com wrote:

In xbmc/cores/dvdplayer/DVDCodecs/Video/StageFrightVideo.cpp:

  • //frame->pts = (dts != DVD_NOPTS_VALUE) ? pts_dtoi(dts) : ((pts != DVD_NOPTS_VALUE) ? pts_dtoi(pts) : 0);
  • frame->pts = (pts != DVD_NOPTS_VALUE) ? pts_dtoi(pts) : ((dts != DVD_NOPTS_VALUE) ? pts_dtoi(dts) : 0);
  • frame->duration = 0;
  • frame->medbuf = p->getBuffer(demuxer_bytes);
  • if (!frame->medbuf)
  • {
  •  free(frame);
    
  •  return VC_ERROR;
    
  • }
  • fast_memcpy(frame->medbuf->data(), demuxer_content, demuxer_bytes);
  • frame->medbuf->meta_data()->clear();
  • frame->medbuf->meta_data()->setInt64(kKeyTime, frame->pts);
  • if (p->mPrevPts >= 0)
  • {
  •  frame->duration = frame->pts - p->mPrevPts;
    

Just ignore gettimesize for now. It's not very important. You should avoid
buffering more than a single frame/packet anyway. Preferably you should not
buffer any, but that likely doesn't work for hw.


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/1832/files#r2891327.

@elupus
elupus Feb 5, 2013 Team Kodi member

Avoid using dts. Never calculate anything from pts, just pass it along. You
may have to re scale it if he expects it in [ms]

Pts should be attached to the buffer passed to the decoder. It will on
output give you the pts back. The order of what you pass into the decoder
is not what you will get out.

On input the pts ts will look jumbled around, but the decoder will reorder
this on output before giving you it back.

Don't try to adjust the timestamp in any way.

@elupus elupus and 1 other commented on an outdated diff Feb 4, 2013
.../cores/dvdplayer/DVDCodecs/Video/StageFrightVideo.cpp
+ pDvdVideoPicture->iLineSize[0] = p->videoStride;
+ pDvdVideoPicture->iLineSize[1] = p->videoStride;
+ pDvdVideoPicture->iLineSize[2] = 0;
+ pDvdVideoPicture->iLineSize[3] = 0;
+ pDvdVideoPicture->data[0] = data;
+ pDvdVideoPicture->data[1] = pDvdVideoPicture->data[0] + (p->videoStride * p->videoSliceHeight);
+ pDvdVideoPicture->data[2] = 0;
+ pDvdVideoPicture->data[3] = 0;
+ break;
+ default:
+ CLog::Log(LOGERROR, "%s::%s - Unsupported color format(%d)\n", CLASSNAME, __func__,p->videoColorFormat);
+ }
+ #if defined(DEBUG_VERBOSE)
+ CLog::Log(LOGDEBUG, ">>> pic pts:%f, data:%p, col:%d, w:%d, h:%d, tm:%d\n", pDvdVideoPicture->pts, data, p->videoColorFormat, p->videoStride, p->videoSliceHeight, XbmcThreads::SystemClockMillis() - time);
+ #endif
+ }
@elupus
elupus Feb 4, 2013 Team Kodi member

You really should avoid any attempt to render here. Pass the frame out from decode down to the normal renderers.

@koying
koying Feb 5, 2013 Team Kodi member

Not sure what you mean by "render", here. I'm just passing the YUV image the normal renderer...

@koying
Member
koying commented Mar 26, 2013

Cleaned up and squashed in #2504 .
Closing this one

@koying koying closed this Mar 26, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment