Skip to content

Commit

Permalink
Annotated a number of issues I found
Browse files Browse the repository at this point in the history
Added some descriptions of the issues I found or what confused me - hopefully this will help anyone else that also found this code as I did.
  • Loading branch information
HowardsPlayPen committed Apr 30, 2021
1 parent 3575836 commit d2a0f15
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 10 deletions.
31 changes: 24 additions & 7 deletions Media.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ HRESULT Media::CreateCaptureDevice()
is selected
*/
//Get a source reader from the first available device
SetSourceReader(devices[0]);
hr = SetSourceReader(devices[0]);

WCHAR *nameString = NULL;
// Get the human-friendly name of the device
Expand All @@ -89,6 +89,10 @@ HRESULT Media::CreateCaptureDevice()

if (SUCCEEDED(hr))
{
/// Note: See https://docs.microsoft.com/en-us/windows/win32/medfound/recommended-8-bit-yuv-formats-for-video-rendering
/// for a good description of the formats.
/// On my computer the webcam support YUY2 which has a stride of 2

//allocate a byte buffer for the raw pixel data
bytesPerPixel = abs(stride) / width;
rawData = new BYTE[width*height * bytesPerPixel];
Expand All @@ -112,6 +116,9 @@ HRESULT Media::SetSourceReader(IMFActivate *device)

EnterCriticalSection(&criticalSection);

// Note: I have found the following call to fail with the error:
// hr 0xc00d36b4 : The data specified for the media type is invalid, inconsistent, or not supported by this object. HRESULT
// So some vidcap devices are not happy with this combination.
hr = device->ActivateObject(__uuidof(IMFMediaSource), (void**)&source);

//get symbolic link for the device
Expand All @@ -132,19 +139,28 @@ HRESULT Media::SetSourceReader(IMFActivate *device)
// Try to find a suitable output type.
if (SUCCEEDED(hr))
{
/// Update: This for loop was aimed at iterating across all supported (native) media types - but there were a couple of strange things which have been updated below
for (DWORD i = 0; ; i++)
{
hr = sourceReader->GetNativeMediaType((DWORD)MF_SOURCE_READER_FIRST_VIDEO_STREAM,i,&mediaType);

/// Note: FAILED() here does not stop on S_FALSE - which is correct, as it should continue in this loop but might not sound logical
/// but if you loop through ALL supported media types the GetNativeMediaType will eventually return a code to indicate there are no more types available
if (FAILED(hr)) { break; }

// Note: I moved this call to here as elsewhere in this code it uses stride without checking if it was zero
//Get the stride for this format so we can calculate the number of bytes per pixel
GetDefaultStride(mediaType, &stride);

hr = IsMediaTypeSupported(mediaType);
if (FAILED(hr)) { break; }
if (FAILED(hr)) { break; } /// Note: this is not detecting S_FALSE
//Get width and height
MFGetAttributeSize(mediaType, MF_MT_FRAME_SIZE, &width, &height);
if (mediaType)
{ mediaType->Release(); mediaType = NULL; }

if (SUCCEEDED(hr))// Found an output type.
/// Note: this SUCCESS(hr) is WRONG - i.e. if 'hr' was S_FALSE (did not find media type) then this still goes ahead thinking it HAD found an output type
if (S_OK == hr)// Found an output type - by explicityly checking for S_OK
break;
}
}
Expand Down Expand Up @@ -177,20 +193,21 @@ HRESULT Media::IsMediaTypeSupported(IMFMediaType *pType)
BOOL bFound = FALSE;
GUID subtype = { 0 };

//Get the stride for this format so we can calculate the number of bytes per pixel
GetDefaultStride(pType, &stride);

if (FAILED(hr)) { return hr; }
hr = pType->GetGUID(MF_MT_SUBTYPE, &subtype);

videoFormat = subtype;

if (FAILED(hr)) {return hr; }

/// Update: The below is still lying - eg NV12 or RGB32 are not supported as there is no NV12 to BGRA function in main.cpp
if (subtype == MFVideoFormat_RGB32 || subtype == MFVideoFormat_RGB24 || subtype == MFVideoFormat_YUY2 || subtype == MFVideoFormat_NV12)
return S_OK;
else
return S_FALSE;
/// Update: Returning S_FALSE here is unclear as SUCCESS(S_FALSE) is actually true
/// See https://docs.microsoft.com/en-us/windows/win32/learnwin32/error-handling-in-com
/// So either return S_FALSE and check for if(S_FALSE == hr) specifically, but not if you use SUCCESS() or FAIL() as was done in this wider code
return S_FALSE;

return hr;
}
Expand Down
2 changes: 1 addition & 1 deletion Media.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class Media : public IMFSourceReaderCallback //this class inhertis from IMFSourc
public:
LONG stride;
int bytesPerPixel;
GUID videoFormat;
GUID videoFormat; // MFVideoFormat_RGB32 / MFVideoFormat_RGB24 / MFVideoFormat_YUY2 / MFVideoFormat_NV12
UINT height;
UINT width;
WCHAR deviceNameString[2048];
Expand Down
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,17 @@ and was a good introduction to accessing Webcams using Microsoft Foundation api

So I thought I would update and annotate the code to show what I found.

This first commit is the base code as was downloaded - ie. prior to any updates and annotations.
1) RGB vs YUY2 NV12 etc

Inside Media::IsMediaTypeSupported it checks if the native media type is one of a list of options, including YUY2 and NV12 BUT it then makes a gross assumption when displaying the pixel data that it is RGB. As others have seen this basically means it will exception nastily (both my webcams natively support YUY2 and so the code did not work out of the box)

1a) I found Media::IsMediaTypeSupported confusing

It returned S_OK for formats that it clearly did not support (as it only supported RGB24).
It returned S_FALSE for everything else - but in Media::SetSourceReader where it called IsMediaSupported it checks for FAILED on this return code (but S_FALSE is still a success in COM speak)

IT ALSO then checks for SUCCESS(hr) to see if it had found a supported media type - but as above it would think SUCCESS(S_FALSE) had worked (!!). So it would stop looping even though it had not found a supported type!

2) I added an implementation of YUY2 to RGBA (TransformImage_YUY2)

I merely referred to an implementation on the Intel website https://software.intel.com/sites/landingpage/mmsf/documentation/preview_8cpp.html#af3d9de67be8e955b824d4b497bba4c96
91 changes: 90 additions & 1 deletion main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <Windows.h>
#include <stdio.h>

#include <iostream>

struct BGRAPixel
{
BYTE b;
Expand All @@ -27,6 +29,69 @@ void RGB24_to_BGRA32(BGRAPixel* destinationBuffer, const BYTE* sourceBuffer, DWO
}

}

__forceinline BYTE Clip(int clr)
{
return (BYTE)(clr < 0 ? 0 : (clr > 255 ? 255 : clr));
}

BGRAPixel ConvertYCrCbToRGB(int y,
int cr,
int cb
)
{
BGRAPixel rgbq;

int c = y - 16;
int d = cb - 128;
int e = cr - 128;

rgbq.r = Clip((298 * c + 409 * e + 128) >> 8);
rgbq.g = Clip((298 * c - 100 * d - 208 * e + 128) >> 8);
rgbq.b = Clip((298 * c + 516 * d + 128) >> 8);

return rgbq;
}

/// Note: this is taken from https://software.intel.com/sites/landingpage/mmsf/documentation/preview_8cpp.html#af3d9de67be8e955b824d4b497bba4c96
/// In the online version (from Intel) the comment below on Byte order was incorrect - although the code was fine (!). Never trust documentation..(?)
void TransformImage_YUY2(BYTE* pDest,
LONG lDestStride,
const BYTE* pSrc,
LONG lSrcStride,
DWORD dwWidthInPixels,
DWORD dwHeightInPixels
)
{
if (pDest == NULL || pSrc == NULL)
return;

for (DWORD y = 0; y < dwHeightInPixels; y++)
{
BGRAPixel* pDestPel = (BGRAPixel*)pDest;
WORD * pSrcPel = (WORD*)pSrc;

for (DWORD x = 0; x < dwWidthInPixels; x += 2)
{
// Byte order is Y0 U0 Y1 V0 /// NOTE: On the Intel site this comment was wrong.

int y0 = (int)LOBYTE(pSrcPel[x]);
int u0 = (int)HIBYTE(pSrcPel[x]);
int y1 = (int)LOBYTE(pSrcPel[x + 1]);
int v0 = (int)HIBYTE(pSrcPel[x + 1]);

pDestPel[x] = ConvertYCrCbToRGB(y0, v0, u0);
pDestPel[x + 1] = ConvertYCrCbToRGB(y1, v0, u0);
}

pSrc += lSrcStride;
pDest += lDestStride;
}

}



int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine,int nCmdShow)
{
//debug console
Expand All @@ -48,7 +113,31 @@ int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLin

while (msg.message != WM_QUIT)
{
RGB24_to_BGRA32(bgraBuffer,m->rawData, m->width, m->height);
if(m->videoFormat == MFVideoFormat_RGB24)
{
RGB24_to_BGRA32(bgraBuffer, m->rawData, m->width, m->height);
}
else if (m->videoFormat == MFVideoFormat_YUY2)
{
// Dest stride in the following call is hardwired to 4 * width because it is RGBA (i.e. 4 bytes per pixel)
TransformImage_YUY2((BYTE*)(bgraBuffer),4* m->width, m->rawData, m->stride, m->width, m->height);
}
else if (m->videoFormat == MFVideoFormat_RGB32)
{
std::cerr << "Unsupported type MFVideoFormat_RGB32" << std::endl;
}
else if (m->videoFormat == MFVideoFormat_NV12)
{
std::cerr << "Unsupported type MFVideoFormat_NV12" << std::endl;
}
else
{
// Houston we have a problem...
/// Note: people are welcome to paste the relevant function here to test
std::cerr << "Unsupported video type" << std::endl;
}


window.Draw((BYTE*)bgraBuffer, m->width, m->height);
while (PeekMessage(&msg, 0, 0, 0, PM_REMOVE))
{
Expand Down

0 comments on commit d2a0f15

Please sign in to comment.