ronantakizawa/codereview-bench huggingface dataset:
Repository: xbmc/xbmc
File: xbmc/utils/BitstreamConverter.cpp (C++)

Code before:
        sequence->fps_rate = fpsrate;
        sequence->fps_scale = fpsscale;
      }
    }
    nal_start = nal_end;
  }

  return changed;
}

#ifdef HAVE_LIBDOVI
// Processes Dolby Vision RPU
//   - Converts to profile 8.1 if `m_convert_dovi` is enabled
//   - Sets level 5 metadata to 0 offsets if `m_setDoviZeroLevel5` is enabled
//
// The returned data must be freed with `dovi_data_free`
// May be NULL if no processing was done or if parsing errored
const DoviData* CBitstreamConverter::processDoviRpu(uint8_t* buf, uint32_t nal_size)
{
  // early exit if no processing option is enabled
  if (!m_convert_dovi && !m_setDoviZeroLevel5)
    return NULL;

  DoviRpuOpaque* rpu = dovi_parse_unspec62_nalu(buf, nal_size);
  const DoviRpuDataHeader* header = dovi_rpu_get_header(rpu);
  const DoviData* rpu_data = NULL;

  int ret = 0;
  bool processed = false;

  if (!header)
  {
    dovi_rpu_free(rpu);
    return rpu_data;
  }

  if (m_convert_dovi && header->guessed_profile == 7)
  {
    ret = dovi_convert_rpu_with_mode(rpu, 2);
    processed = true;
  }

  if (ret == 0 && m_setDoviZeroLevel5)
  {
    ret = dovi_rpu_set_active_area_offsets(rpu, 0, 0, 0, 0);
    processed = true;
  }

  if (ret == 0 && processed)
    rpu_data = dovi_write_unspec62_nalu(rpu);


Review comment (suggestion):
```suggestion
  const DoviData* rpuData = NULL;
```

Code after:
        sequence->fps_rate = fpsrate;
        sequence->fps_scale = fpsscale;
      }
    }
    nal_start = nal_end;
  }

  return changed;
}

#ifdef HAVE_LIBDOVI
// Processes Dolby Vision RPU
//   - Converts to profile 8.1 if `m_convert_dovi` is enabled
//   - Sets level 5 metadata to 0 offsets if `m_setDoviZeroLevel5` is enabled
//
// The returned data must be freed with `dovi_data_free`
// May be NULL if no processing was done or if parsing errored
const DoviData* CBitstreamConverter::processDoviRpu(uint8_t* buf, uint32_t nalSize)
{
  // early exit if no processing option is enabled
  if (!m_convert_dovi && !m_setDoviZeroLevel5)
    return NULL;

  DoviRpuOpaque* rpu = dovi_parse_unspec62_nalu(buf, nalSize);
  const DoviRpuDataHeader* header = dovi_rpu_get_header(rpu);
  const DoviData* rpuData = NULL;

  int ret = 0;
  bool processed = false;

  if (!header)
  {
    dovi_rpu_free(rpu);
    return rpuData;
  }

  if (m_convert_dovi && header->guessed_profile == 7)
  {
    ret = dovi_convert_rpu_with_mode(rpu, 2);
    processed = true;
  }

  if (ret == 0 && m_setDoviZeroLevel5)
  {
    ret = dovi_rpu_set_active_area_offsets(rpu, 0, 0, 0, 0);
    processed = true;
  }

  if (ret == 0 && processed)
    rpuData = dovi_write_unspec62_nalu(rpu);
Repository: xbmc/xbmc
File: xbmc/platform/linux/WebOSTVPlatformConfig.cpp (C++)

Code before:
  if (HLunaServiceCall(LUNA_GET_CONFIG, payload.c_str(), &requestContext))
  {
    CLog::LogF(LOGWARNING, "Luna get config request call failed");
  }
}

int WebOSTVPlatformConfig::GetWebOSVersion()
{
  return ms_config[PLATFORM_CODE].asInteger();
}

bool WebOSTVPlatformConfig::SupportsDTS()
{
  return ms_config[EDID_TYPE].asString().find(DTS) != std::string::npos;
}

bool WebOSTVPlatformConfig::SupportsHDR()
{
  return ms_config[SUPPORT_HDR].asBoolean();
}

void WebOSTVPlatformConfig::LoadARCStatus()
{
  HContext requestContext;
  requestContext.pub = true;
  requestContext.multiple = false;
  requestContext.callback = [](LSHandle* sh, LSMessage* msg, void* ctx) -> bool
  {
    std::string message = HLunaServiceMessage(msg);
    CLog::LogF(LOGDEBUG, "ARC controller: {}", message);

    CVariant parsed;
    if (!CJSONVariantParser::Parse(message, parsed))
    {
      CLog::LogF(LOGERROR, "Failed to parse ARC controller JSON");
      return false;
    }

    const std::string arcActivation = parsed[EARC_ACTIVATION].asString();
    bool ddplusSupported = false;

    if (arcActivation == EARC)
    {
      const CVariant& caps = parsed[EARC_CAPABILITY];
      if (caps.isArray())
      {
        for (unsigned int i = 0; i < caps.size(); ++i)
        {
          if (caps[i].asString() == DDPLUS)
          {
            ddplusSupported = true;

Review comment (suggestion):
The luna request should be `multiple = true` so we get notified if the ARC status changes. So basically make the context a member variable so it's not queried again whenever the function is called.

Code after:
  if (HLunaServiceCall(LUNA_GET_CONFIG, payload.c_str(), &requestContext))
  {
    CLog::LogF(LOGWARNING, "Luna get config request call failed");
  }
}

int WebOSTVPlatformConfig::GetWebOSVersion()
{
  return ms_config[PLATFORM_CODE].asInteger();
}

bool WebOSTVPlatformConfig::SupportsDTS()
{
  return ms_config[EDID_TYPE].asString().find(DTS) != std::string::npos;
}

bool WebOSTVPlatformConfig::SupportsHDR()
{
  return ms_config[SUPPORT_HDR].asBoolean();
}

void WebOSTVPlatformConfig::LoadARCStatus()
{
  m_requestContextARC->pub = true;
  m_requestContextARC->multiple = true;
  m_requestContextARC->callback = [](LSHandle* sh, LSMessage* msg, void* ctx) -> bool
  {
    auto* self = static_cast<WebOSTVPlatformConfig*>(ctx);
    std::string message = HLunaServiceMessage(msg);
    CLog::LogF(LOGDEBUG, "ARC controller: {}", message);

    CVariant parsed;
    if (!CJSONVariantParser::Parse(message, parsed))
    {
      CLog::LogF(LOGERROR, "Failed to parse ARC controller JSON");
      return false;
    }

    const std::string arcActivation = parsed[ARC_ACTIVATION].asString();

    if (arcActivation == EARC)
    {
      const CVariant& caps = parsed[EARC_CAPABILITY];
      self->m_eAC3Supported =
          caps.isArray() && std::any_of(caps.begin_array(), caps.end_array(),
                                        [](const CVariant& v) { return v.asString() == DDPLUS; });
    }

    return true;
  };
Repository: xbmc/xbmc
File: xbmc/platform/linux/WebOSTVPlatformConfig.cpp (C++)

Code before:
    std::string message = HLunaServiceMessage(msg);
    CLog::LogF(LOGDEBUG, "ARC controller: {}", message);

    CVariant parsed;
    if (!CJSONVariantParser::Parse(message, parsed))
    {
      CLog::LogF(LOGERROR, "Failed to parse ARC controller JSON");
      return false;
    }

    const std::string arcActivation = parsed[EARC_ACTIVATION].asString();
    bool ddplusSupported = false;

    if (arcActivation == EARC)
    {
      const CVariant& caps = parsed[EARC_CAPABILITY];
      if (caps.isArray())
      {
        for (unsigned int i = 0; i < caps.size(); ++i)
        {
          if (caps[i].asString() == DDPLUS)
          {
            ddplusSupported = true;
            break;
          }
        }
      }
    }

    ms_config[SUPPORT_EARCDDPlus] = ddplusSupported;

    return true;
  };

  std::string payload = "{}";
  constexpr const char* LUNA_ARC_STATUS = "luna://com.webos.service.arccontroller/getARCState";

  if (HLunaServiceCall(LUNA_ARC_STATUS, payload.c_str(), &requestContext))
  {
    CLog::LogF(LOGWARNING, "Luna ARC controller request call failed");
  }
}

bool WebOSTVPlatformConfig::SupportsEARCDDPlus()
{
  LoadARCStatus();
  return ms_config[SUPPORT_EARCDDPlus].asBoolean();
}


Review comment (suggestion):
~~Can this not use `std::ranges::contains`?~~
Use `std::find` in C++20

Code after:
    std::string message = HLunaServiceMessage(msg);
    CLog::LogF(LOGDEBUG, "ARC controller: {}", message);

    CVariant parsed;
    if (!CJSONVariantParser::Parse(message, parsed))
    {
      CLog::LogF(LOGERROR, "Failed to parse ARC controller JSON");
      return false;
    }

    const std::string arcActivation = parsed[ARC_ACTIVATION].asString();

    if (arcActivation == EARC)
    {
      const CVariant& caps = parsed[EARC_CAPABILITY];
      self->m_eAC3Supported =
          caps.isArray() && std::any_of(caps.begin_array(), caps.end_array(),
                                        [](const CVariant& v) { return v.asString() == DDPLUS; });
    }

    return true;
  };

  if (HLunaServiceCall(LUNA_ARC_STATUS, "{}", m_requestContextARC.get()))
  {
    CLog::LogF(LOGWARNING, "Luna ARC controller request call failed");
  }
}

bool WebOSTVPlatformConfig::SupportsEAC3()
{
  return m_eAC3Supported;
}
Repository: xbmc/xbmc
File: xbmc/platform/linux/WebOSTVPlatformConfig.cpp (C++)

Code before:
    if (!CJSONVariantParser::Parse(message, parsed))
    {
      CLog::LogF(LOGERROR, "Failed to parse ARC controller JSON");
      return false;
    }

    const std::string arcActivation = parsed[EARC_ACTIVATION].asString();
    bool ddplusSupported = false;

    if (arcActivation == EARC)
    {
      const CVariant& caps = parsed[EARC_CAPABILITY];
      if (caps.isArray())
      {
        for (unsigned int i = 0; i < caps.size(); ++i)
        {
          if (caps[i].asString() == DDPLUS)
          {
            ddplusSupported = true;
            break;
          }
        }
      }
    }

    ms_config[SUPPORT_EARCDDPlus] = ddplusSupported;

    return true;
  };

  std::string payload = "{}";
  constexpr const char* LUNA_ARC_STATUS = "luna://com.webos.service.arccontroller/getARCState";

  if (HLunaServiceCall(LUNA_ARC_STATUS, payload.c_str(), &requestContext))
  {
    CLog::LogF(LOGWARNING, "Luna ARC controller request call failed");
  }
}

bool WebOSTVPlatformConfig::SupportsEARCDDPlus()
{
  LoadARCStatus();
  return ms_config[SUPPORT_EARCDDPlus].asBoolean();
}


Review comment (suggestion):
Please don't use `ms_config` and parse into a dedicated member `std::atomic<bool>`

Code after:
    if (!CJSONVariantParser::Parse(message, parsed))
    {
      CLog::LogF(LOGERROR, "Failed to parse ARC controller JSON");
      return false;
    }

    const std::string arcActivation = parsed[ARC_ACTIVATION].asString();

    if (arcActivation == EARC)
    {
      const CVariant& caps = parsed[EARC_CAPABILITY];
      self->m_eAC3Supported =
          caps.isArray() && std::any_of(caps.begin_array(), caps.end_array(),
                                        [](const CVariant& v) { return v.asString() == DDPLUS; });
    }

    return true;
  };

  if (HLunaServiceCall(LUNA_ARC_STATUS, "{}", m_requestContextARC.get()))
  {
    CLog::LogF(LOGWARNING, "Luna ARC controller request call failed");
  }
}

bool WebOSTVPlatformConfig::SupportsEAC3()
{
  return m_eAC3Supported;
}
Repository: xbmc/xbmc
File: xbmc/platform/linux/WebOSTVPlatformConfig.cpp (C++)

Code before:
/*
 *  Copyright (C) 2025 Team Kodi
 *  This file is part of Kodi - https://kodi.tv
 *
 *  SPDX-License-Identifier: GPL-2.0-or-later
 *  See LICENSES/README.md for more information.
 */

#include "WebOSTVPlatformConfig.h"

#include "utils/JSONVariantParser.h"
#include "utils/JSONVariantWriter.h"
#include "utils/Variant.h"
#include "utils/log.h"

HContext WebOSTVPlatformConfig::m_requestContextARC{};
std::atomic<bool> WebOSTVPlatformConfig::m_ddPlusSupported{false};

namespace
{
constexpr const char* LUNA_GET_CONFIG = "luna://com.webos.service.config/getConfigs";

constexpr const char* CONFIGS = "configs";
constexpr const char* CONFIG_NAMES = "configNames";
const auto QUERY_CONFIG = std::vector<std::string>{"tv.model.*", "tv.nyx.*"};

constexpr const char* EDID_TYPE = "tv.model.edidType";
constexpr const char* SUPPORT_HDR = "tv.model.supportHDR";

constexpr const char* PLATFORM_CODE = "tv.nyx.platformCode";

constexpr const char* DTS = "dts";

constexpr const char* LUNA_ARC_STATUS = "luna://com.webos.service.arccontroller/getARCState";

constexpr const char* ARC_ACTIVATION = "arcActivation";
constexpr const char* EARC_CAPABILITY = "earcCapability";
constexpr const char* SUPPORT_EARCDDPlus = "eARCDDPlus";
constexpr const char* EARC = "eARC";
constexpr const char* DDPLUS = "DD+";


Review comment (suggestion):
For a proper cleanup we need to call `HUnregisterServiceCallback` when finished. The easiest way would be to wrap this in a `std::unique_ptr` with a custom destructor that calls the function. Also declare as static inline to avoid the definition

Code after:
/*
 *  Copyright (C) 2025 Team Kodi
 *  This file is part of Kodi - https://kodi.tv
 *
 *  SPDX-License-Identifier: GPL-2.0-or-later
 *  See LICENSES/README.md for more information.
 */

#include "WebOSTVPlatformConfig.h"

#include "utils/JSONVariantParser.h"
#include "utils/JSONVariantWriter.h"
#include "utils/Variant.h"
#include "utils/log.h"

namespace
{
constexpr const char* LUNA_GET_CONFIG = "luna://com.webos.service.config/getConfigs";

constexpr const char* CONFIGS = "configs";
constexpr const char* CONFIG_NAMES = "configNames";
const auto QUERY_CONFIG = std::vector<std::string>{"tv.model.*", "tv.nyx.*"};

constexpr const char* EDID_TYPE = "tv.model.edidType";
constexpr const char* SUPPORT_HDR = "tv.model.supportHDR";

constexpr const char* PLATFORM_CODE = "tv.nyx.platformCode";

constexpr const char* DTS = "dts";

constexpr const char* LUNA_ARC_STATUS = "luna://com.webos.service.arccontroller/getARCState";

constexpr const char* ARC_ACTIVATION = "arcActivation";
constexpr const char* EARC_CAPABILITY = "earcCapability";
constexpr const char* SUPPORT_EARCDDPlus = "eARCDDPlus";
constexpr const char* EARC = "eARC";
constexpr const char* DDPLUS = "DD+";

CVariant ms_config;
} // namespace
Repository: xbmc/xbmc
File: xbmc/platform/linux/WebOSTVPlatformConfig.cpp (C++)

Code before:
  {
    auto* self = static_cast<WebOSTVPlatformConfig*>(ctx);
    std::string message = HLunaServiceMessage(msg);
    CLog::LogF(LOGDEBUG, "ARC controller: {}", message);

    CVariant parsed;
    if (!CJSONVariantParser::Parse(message, parsed))
    {
      CLog::LogF(LOGERROR, "Failed to parse ARC controller JSON");
      return false;
    }

    const std::string arcActivation = parsed[ARC_ACTIVATION].asString();

    if (arcActivation == EARC)
    {
      const CVariant& caps = parsed[EARC_CAPABILITY];
      if (caps.isArray())
      {
        auto it = std::find_if(caps.begin_array(), caps.end_array(),
                               [](const CVariant& v) { return v.asString() == DDPLUS; });
        if (it != caps.end_array())
        {
          self->m_ddPlusSupported = true;
        }
      }
    }

    return true;
  };

  if (HLunaServiceCall(LUNA_ARC_STATUS, "{}", &m_requestContextARC))
  {
    CLog::LogF(LOGWARNING, "Luna ARC controller request call failed");
  }
}

bool WebOSTVPlatformConfig::SupportsEARCDDPlus()
{
  return m_ddPlusSupported;
}


Review comment (suggestion):
```suggestion

      self->m_ddPlusSupported = caps.isArray() && std::any_of(caps.begin_array(), caps.end_array(),
                                              [](const CVariant& v) { return v.asString() == DDPLUS; });
```

Code after:
    CLog::LogF(LOGDEBUG, "ARC controller: {}", message);

    CVariant parsed;
    if (!CJSONVariantParser::Parse(message, parsed))
    {
      CLog::LogF(LOGERROR, "Failed to parse ARC controller JSON");
      return false;
    }

    const std::string arcActivation = parsed[ARC_ACTIVATION].asString();

    if (arcActivation == EARC)
    {
      const CVariant& caps = parsed[EARC_CAPABILITY];
      self->m_eAC3Supported =
          caps.isArray() && std::any_of(caps.begin_array(), caps.end_array(),
                                        [](const CVariant& v) { return v.asString() == DDPLUS; });
    }

    return true;
  };

  if (HLunaServiceCall(LUNA_ARC_STATUS, "{}", m_requestContextARC.get()))
  {
    CLog::LogF(LOGWARNING, "Luna ARC controller request call failed");
  }
}

bool WebOSTVPlatformConfig::SupportsEAC3()
{
  return m_eAC3Supported;
}
Repository: xbmc/xbmc
File: xbmc/platform/linux/WebOSTVPlatformConfig.h (C/C++)

Code before:
 *  See LICENSES/README.md for more information.
 */

#pragma once

#include <atomic>
#include <memory>

#include <webos-helpers/libhelpers.h>

class CVariant;

struct HContextDeleter
{
  void operator()(HContext* ctx) const noexcept
  {
    if (ctx)
      HUnregisterServiceCallback(ctx);
  }
};

class WebOSTVPlatformConfig
{
private:
  static inline std::unique_ptr<HContext, HContextDeleter> m_requestContextARC{new HContext{}};
  static inline std::atomic<bool> m_eAC3Supported;

public:
  static void Load();
  static void LoadARCStatus();
  static int GetWebOSVersion();
  static bool SupportsDTS();
  static bool SupportsHDR();
  static bool SupportsEAC3();
};


Review comment (suggestion):
Initialize with `false` for clarity, even though it's not strictly necessary
```suggestion
  static inline std::atomic<bool> m_eAC3Supported{false};
```

Code after:
 *  See LICENSES/README.md for more information.
 */

#pragma once

#include <atomic>
#include <memory>

#include <webos-helpers/libhelpers.h>

class CVariant;

struct HContextDeleter
{
  void operator()(HContext* ctx) const noexcept { HUnregisterServiceCallback(ctx); }
};

class WebOSTVPlatformConfig
{
private:
  static inline std::unique_ptr<HContext, HContextDeleter> m_requestContextARC{new HContext{}};
  static inline std::atomic<bool> m_eAC3Supported{false};

public:
  static void Load();
  static void LoadARCStatus();
  static int GetWebOSVersion();
  static bool SupportsDTS();
  static bool SupportsHDR();
  static bool SupportsEAC3();
};
Repository: xbmc/xbmc
File: xbmc/interfaces/json-rpc/PlayerOperations.cpp (C++)

Code before:
  // Return the chapters list of the running video
  switch (GetPlayer(parameterObject["playerid"]))
  {
    case Video:
      break;
    default: return InvalidParams;
  }
  auto& components = CServiceBroker::GetAppComponents();
  const auto appPlayer = components.GetComponent<CApplicationPlayer>();

  if (!appPlayer->IsPlayingVideo())
    return FailedToExecute; // No running video

  // Extract chapters from CApplicationPlayer
  int chapterCount = appPlayer->GetChapterCount();
  CVariant chapters(CVariant::VariantTypeArray);

  for (int i = 0; i < chapterCount; ++i)
  {
    CVariant chapter(CVariant::VariantTypeObject);
    chapter["index"] = i;
    // Chapter name
    std::string name;
    appPlayer->GetChapterName(name, i + 1);
    if (!name.empty())
      chapter["name"] = name;
    // Chapter position in seconds
    int64_t position = appPlayer->GetChapterPos(i + 1);
    if (position < 0) continue;
    chapter["time"] = position;
    chapters.push_back(chapter);
  }

  result["chapters"] = chapters;
  return OK;
}


Review comment (suggestion):
The description says one based but you are zero based here. I'd suggest to make the whole loop  one based:
```cpp
for (int i = 1; i <= chapterCount; ++i)
```

Code after:
                                              IClient* client,
                                              const CVariant& parameterObject,
                                              CVariant& result)
{
  // Return the chapters list of the running video or empty list if none
  switch (GetPlayer(parameterObject["playerid"]))
  {
    case Video:
      break;
    default:
      return InvalidParams;
  }
  auto& components = CServiceBroker::GetAppComponents();
  const auto appPlayer = components.GetComponent<CApplicationPlayer>();

  if (!appPlayer->IsPlayingVideo())
    return FailedToExecute; // No running video

  // Extract chapters from CApplicationPlayer
  const int chapterCount = appPlayer->GetChapterCount();
  CVariant chapters(CVariant::VariantTypeArray);

  for (int i = 1; i <= chapterCount; ++i)
  {
    CVariant chapter(CVariant::VariantTypeObject);
    chapter["index"] = i;
    // Chapter name
    std::string name;
    appPlayer->GetChapterName(name, i);
    if (!name.empty())
      chapter["name"] = name;
    // Chapter position in seconds
    int64_t position = appPlayer->GetChapterPos(i);
    if (position < 0)
      continue;
    chapter["time"] = position;
    chapters.push_back(chapter);
  }

  result["chapters"] = std::move(chapters);
  return OK;
}
Repository: xbmc/xbmc
File: xbmc/interfaces/json-rpc/PlayerOperations.cpp (C++)

Code before:
  const auto appPlayer = components.GetComponent<CApplicationPlayer>();

  if (!appPlayer->IsPlayingVideo())
    return FailedToExecute; // No running video

  // Extract chapters from CApplicationPlayer
  int chapterCount = appPlayer->GetChapterCount();
  CVariant chapters(CVariant::VariantTypeArray);

  for (int i = 0; i < chapterCount; ++i)
  {
    CVariant chapter(CVariant::VariantTypeObject);
    chapter["index"] = i;
    // Chapter name
    std::string name;
    appPlayer->GetChapterName(name, i + 1);
    if (!name.empty())
      chapter["name"] = name;
    // Chapter position in seconds
    int64_t position = appPlayer->GetChapterPos(i + 1);
    if (position < 0) continue;
    chapter["time"] = position;
    chapters.push_back(chapter);
  }

  result["chapters"] = chapters;
  return OK;
}


Review comment (suggestion):
```suggestion
  result["chapters"] = std::move(chapters);
```

Code after:
      break;
    default:
      return InvalidParams;
  }
  auto& components = CServiceBroker::GetAppComponents();
  const auto appPlayer = components.GetComponent<CApplicationPlayer>();

  if (!appPlayer->IsPlayingVideo())
    return FailedToExecute; // No running video

  // Extract chapters from CApplicationPlayer
  const int chapterCount = appPlayer->GetChapterCount();
  CVariant chapters(CVariant::VariantTypeArray);

  for (int i = 1; i <= chapterCount; ++i)
  {
    CVariant chapter(CVariant::VariantTypeObject);
    chapter["index"] = i;
    // Chapter name
    std::string name;
    appPlayer->GetChapterName(name, i);
    if (!name.empty())
      chapter["name"] = name;
    // Chapter position in seconds
    int64_t position = appPlayer->GetChapterPos(i);
    if (position < 0)
      continue;
    chapter["time"] = position;
    chapters.push_back(chapter);
  }

  result["chapters"] = std::move(chapters);
  return OK;
}