911113 - Ambiguity in checking the resolution capability of an OMX decoder

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Bruce Sun [:brsun]]

There are two blocks of codes to check OMX decoder capability with the required frame size:
 - During the creation of an OMX decoder component, we need to configure input port and output port of it. If IOMX::setParameter() fails to configure resolution of input/output port, the created OMXCodec object will be destroyed.
 - After the created OMXCodec object can be configured successfully, we will use values of "ro.moz.omx.hw.max_width" and "ro.moz.omx.hw.max_height" to check with the required frame width/height. If the required frame width*height exceeds predefined max_width*max_height, we will not use this OMXCodec object for decoding.

The logic of the second block of codes seems like some porting/customization of underlying HW codec. Is it good to move this logic into underlying OMX component so that we can just use the first block of codes to check the capability of the decoder? Or the second logic is required to avoid some error conditions which cannot be checked through IOMX::setPrameter() API?

Kindly share your information on this.

// First block of codes:
status_t OMXCodec::setVideoOutputFormat(const char *mime, OMX_U32 width, OMX_U32 height) {
  .....
  OMX_PARAM_PORTDEFINITIONTYPE def;
  InitOMXParams(&def);
  def.nPortIndex = kPortIndexInput;
  .....
  video_def->nFrameWidth = width;
  video_def->nFrameHeight = height;
  .....
  err = mOMX->setParameter(mNode, OMX_IndexParamPortDefinition, &def, sizeof(def));
  if (err != OK) {
    return err;
  }
  .....
  InitOMXParams(&def);
  def.nPortIndex = kPortIndexOutput;
  .....
  video_def->nFrameWidth = width;
  video_def->nFrameHeight = height;
  err = mOMX->setParameter(mNode, OMX_IndexParamPortDefinition, &def, sizeof(def));
  return err;
}

// Second block of codes:
void OMXCodecProxy::statusChanged(int event)
{
  .....
  if (!strncasecmp(mime, "video/", 6)) {
    .....
    // Check if this video is sized such that we're comfortable
    // possibly using an OMX decoder.
    int32_t maxWidth, maxHeight;
    char propValue[PROPERTY_VALUE_MAX];
    property_get("ro.moz.omx.hw.max_width", propValue, "-1");
    maxWidth = atoi(propValue);
    property_get("ro.moz.omx.hw.max_height", propValue, "-1");
    maxHeight = atoi(propValue);

    int32_t width = -1, height = -1;
    if (maxWidth > 0 && maxHeight > 0 &&
        !(mOMXCodec->getFormat()->findInt32(kKeyWidth, &width) &&
          mOMXCodec->getFormat()->findInt32(kKeyHeight, &height) &&
          width * height <= maxWidth * maxHeight)) {
      printf_stderr("Failed to get video size, or it was too large for HW decoder (<w=%d, h=%d> but <maxW=%d, maxH=%d>)",
                    width, height, maxWidth, maxHeight);
      mOMXCodec.clear();
      mState = MediaResourceManagerClient::CLIENT_STATE_SHUTDOWN;
      notifyStatusChangedLocked();
      return;
    }
    .....
  }
 .....
}

Comment 1

[Sotaro Ikeda [:sotaro]]

Bug 829361 comment #3 explains about it. Even when Hw codec can support the some large size video, there is a cases that needs to limit the size of decoding like resource limitation. OMXCodec is outside of gecko, keep "ro.moz.omx.hw.max_width" in gecko is convenient. It might be better to add a comment here.

Comment 2

[Marco Chen [:mchen]]

Is there any place to note this kind of thing? It needs every partner to put correct value on their different devices equipped different resources.

Basically I saw it it VGA on Leo and WVGA on Helix. Does the limitation is always device's screen size?

Thanks.

Comment 3

[Sotaro Ikeda [:sotaro]]

(In reply to Marco Chen [:mchen] from comment #2)
> Basically I saw it it VGA on Leo and WVGA on Helix. Does the limitation is
> always device's screen size?

It should be decided by partners. Actually the product need to support at least device's screen size, otherwise it could play only blurry videos.

Comment 4

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> 
> It should be decided by partners. Actually the product need to support at
> least device's screen size, otherwise it could play only blurry videos.

Correction;
Video becomes always blurry when play backed in fullscreen, if the product support smaller video side than device's screen size.

Comment 5

[Sotaro Ikeda [:sotaro]]

(In reply to Marco Chen [:mchen] from comment #2)
> Is there any place to note this kind of thing? It needs every partner to put
> correct value on their different devices equipped different resources.

:mwu, is there a place in b2g source for gathering and explaining the b2g specific property like "ro.moz.omx.hw.max_width".

Comment 6

[Michael Wu [:mwu]]

(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> :mwu, is there a place in b2g source for gathering and explaining the b2g
> specific property like "ro.moz.omx.hw.max_width".

Not that I know of, but we should create one. I would create a wiki.mozilla page for it - not sure if there's a good place in the source for it since any piece of code can query those properties.

Comment 7

[Blake Wu [:bwu][:blakewu]]

This bug is hit a couple of times when porting to some new devices and spend some R&D resource to debug it. It would be better to remove it. AFAIK, Android doesn't have this kind of check. 
If this check is really required, partners can add it to their OMX HAL to return an error when setting parameter as first block codes mentioned comment 0.

Comment 8

[BMO Automation]

Firefox OS is not being worked on