Skip to content

Conversation

@xiaozhuai
Copy link

@xiaozhuai xiaozhuai commented Apr 21, 2025

Description

directshow claims that the color format supported is MEDIASUBTYPE_RGB24, and it should correspond to VideoFormat::RGB24 instead of VideoFormat::XRGB

Motivation and Context

This change is completely self-explanatory

How Has This Been Tested?

I have wrote the following test code and tested multiple camera devices that support RGB24 format on Windows 10, and now it can provide the correct format.

#include <ole2.h>
#include <cstdio>
#include "dshowcapture.hpp"

const char *format2str(DShow::VideoFormat format) {
    switch (format) {
        case DShow::VideoFormat::Any:
            return "Any";
        default:
        case DShow::VideoFormat::Unknown:
            return "Unknown";
        case DShow::VideoFormat::ARGB:
            return "ARGB";
        case DShow::VideoFormat::XRGB:
            return "XRGB";
        case DShow::VideoFormat::RGB24:
            return "RGB";
        case DShow::VideoFormat::I420:
            return "I420";
        case DShow::VideoFormat::NV12:
            return "NV12";
        case DShow::VideoFormat::YV12:
            return "YV12";
        case DShow::VideoFormat::Y800:
            return "Y800";
        case DShow::VideoFormat::YVYU:
            return "YVYU";
        case DShow::VideoFormat::YUY2:
            return "YUY2";
        case DShow::VideoFormat::UYVY:
            return "UYVY";
        case DShow::VideoFormat::HDYC:
            return "HDYC";
        case DShow::VideoFormat::MJPEG:
            return "MJPEG";
        case DShow::VideoFormat::H264:
            return "H264";
    }
}

int main() {
    CoInitialize(nullptr);

    std::vector<DShow::VideoDevice> devices;
    if (!DShow::Device::EnumVideoDevices(devices)) {
        printf("ERROR! Unable to list video devices\n");
        return -1;
    }

    if (devices.empty()) {
        printf("ERROR! No video devices found\n");
        return -1;
    }

    printf("Devices:\n");
    for (int i = 0; i < devices.size(); i++) {
        const auto &device = devices[i];
        printf("----------------------------------\n");
        printf("- Device %d:\n", i);
        wprintf(L"  - Device: %s\n", device.name.c_str());
        wprintf(L"  - Path: %s\n", device.path.c_str());
        printf("  - Caps:\n");
        for (const auto &cap : device.caps) {
            printf("    - %dx%d %s\n", cap.minCX, cap.minCY, format2str(cap.format));
        }
    }

    return 0;
}

Types of changes

Bug fix

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@Fenrirthviti
Copy link
Member

Please properly fill out the PR template.

@BillyONeal
Copy link

else if (mt.subtype == MEDIASUBTYPE_IYUV)
format = VideoFormat::I420;

also looks like it might be a copy pasto. (I have no idea what this code does, I'm just looking for the pattern that the format 'kind' seems to usually match elsewhere in this file)

@BillyONeal
Copy link

Possibly related: 5ac25cb

@Fenrirthviti Can you clarify what you mean by PR template?
@xiaozhuai Can you try to follow the PR template?

@Fenrirthviti
Copy link
Member

I'm not sure what needs clarifying. Our repo requires all PRs to use our PR template, provided as part of the GitHub repo config. I'm baffled how this was even submitted without using one, as it's required to submit a PR to us. It was either using some third-party client that bypasses this repo setting, or the user explicitly looked at the template and chose to delete it.

@xiaozhuai
Copy link
Author

@BillyONeal @Fenrirthviti Ok guys, I've filled out the PR template.

@BillyONeal
Copy link

I'm not sure what needs clarifying. Our repo requires all PRs to use our PR template, provided as part of the GitHub repo config.

I was looking for a .github/pull_request_template.md but didn't find one 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants