Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#3447 closed defect (fixed)

dshow does not set codec_tag, required for YVU vs YUV

Reported by: DonMoir Owned by:
Priority: normal Component: avdevice
Version: git-master Keywords: dshow
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

As far as I can tell, I420 and IYUV reverses the U and V planes from YV12 and there doesn't seem to be a mapping for YV12 other than YUV420p or vice versa whatever the case may be I think they are supposed to be reversed.

I have a software DirectShow screen capture device MediaLooks. It has both I420 and YV12 formats and some others. YV12 is listed before I420 and since they both map to YUV420P seems it's confused and the colors are reversed.

See: http://www.fourcc.org/yuv.php#YV12

first few pixel formats in map in libavcodec/raw.c

const PixelFormatTag ff_raw_pix_fmt_tags[] = {
     { AV_PIX_FMT_YUV420P, MKTAG('I', '4', '2', '0') }, /* Planar formats */
     { AV_PIX_FMT_YUV420P, MKTAG('I', 'Y', 'U', 'V') },
     { AV_PIX_FMT_YUV420P, MKTAG('Y', 'V', '1', '2') },
     { AV_PIX_FMT_YUV410P, MKTAG('Y', 'U', 'V', '9') },
     { AV_PIX_FMT_YUV410P, MKTAG('Y', 'V', 'U', '9') },

Attachments (1)

patchdshowcodec_tag.diff (612 bytes ) - added by Carl Eugen Hoyos 10 years ago.

Download all attachments as: .zip

Change History (6)

by Carl Eugen Hoyos, 10 years ago

Attachment: patchdshowcodec_tag.diff added

comment:1 by Carl Eugen Hoyos, 10 years ago

Component: undeterminedavdevice
Keywords: dshow added
Version: unspecifiedgit-master

Please test attached patch.

comment:2 by DonMoir, 10 years ago

That worked for dshow. Not sure what would happen in the normal file case though.

Sorry to put this here but it's quick and on the way to bed.

BI_RGB indicates opaque RGB and not alpha RGB. I think same for BI_BITFIELDS. Even if 32 bit.

dshow.c

static enum AVPixelFormat dshow_pixfmt(DWORD biCompression, WORD biBitCount)
{
    switch(biCompression) {
    case BI_BITFIELDS:
    case BI_RGB:
        switch(biBitCount) { /* 1-8 are untested */
            case 1:
                return AV_PIX_FMT_MONOWHITE;
            case 4:
                return AV_PIX_FMT_RGB4;
            case 8:
                return AV_PIX_FMT_RGB8;
            case 16:
                return AV_PIX_FMT_RGB555;
            case 24:
                return AV_PIX_FMT_BGR24;
            case 32:
-                return AV_PIX_FMT_RGB32;
+                return AV_PIX_FMT_0RGB32;
        }
    }
    return avpriv_find_pix_fmt(ff_raw_pix_fmt_tags, biCompression); // all others
}
Last edited 10 years ago by DonMoir (previous) (diff)

comment:3 by Carl Eugen Hoyos, 10 years ago

Resolution: fixed
Status: newclosed
Summary: YV12 and I420 and IYUV all map to YUV420Pdshow does not set codec_tag, required for YVU vs YUV

Thank you for testing.

For future tickets: Please remember to always post a failing command line together with complete, uncut console output for issues that are reproducible with ffmpeg. You may add your analysis but please remember that it can be confusing (like in this case), please do not replace the report with an analysis to save us all some time.
Concerning patches, please note that only unified diffs (as supported by GNU patch) are accepted for less trivial changes and please send all patches to ffmpeg-devel, patches on the bug tracker are often ignored.

comment:4 by DonMoir, 10 years ago

I would have posted the command line but at the time I posted this bug, ffmpeg and ffplay did not work at all with this particular driver. There is another bug in dshow code I am looking into and I have that working now but it's a quick fix and looking for real solution or at least verification.

Not too concerned with above RGB fix and just put that up as notification as I was about to pass out. You can see the comment /* 1-8 untested */ apparently 32 was not tested properly either. Easy to miss though since tester was probably not testing with alpha surfaces. The way that AV_PIX_FMT_RGB32 is hidden with alpha is not so cool either.

comment:5 by Roger Pack, 10 years ago

if Don has tested and it fixes some problem, then LGTM.

Note: See TracTickets for help on using tickets.