ronantakizawa/codereview-bench huggingface dataset:
Repository: xbmc/xbmc
File: xbmc/cores/VideoPlayer/Edl.h (C/C++)

Code before:
  std::vector<EDL::Edit> m_vecEdits;
  std::vector<int> m_vecSceneMarkers;

  /*!
   * @brief Last processed EDL edit time (ms)
  */
  int m_lastEditTime;

  /*!
   * @brief Last processed EDL edit action type
  */
  EDL::Action m_lastEditActionType{EDL::EDL_ACTION_NONE};

  bool ReadEdl(const std::string& mediaFilePath, float fps);

  bool ReadComskip(const std::string& mediaFilePath, float fps);

  bool ReadVideoReDo(const std::string& mediaFilePath);

  bool ReadBeyondTV(const std::string& mediaFilePath);

  bool ReadPvr(const CFileItem& fileItem);

  /*!
   * @brief Adds an edit to the list of EDL edits
   * @param newEdit the edit to add
   * @return true if the operation succeeds, false otherwise
  */
  bool AddEdit(const EDL::Edit& newEdit);

  bool AddSceneMarker(int sceneMarker);

  void MergeShortCommBreaks();

  /*!
   * @brief Adds scene markers at the start and end of some edits
   * (currently only for commercial breaks)
  */
  void AddSceneMarkersAtStartAndEndOfEdits();
};


Review comment (suggestion):
```suggestion
  bool ReadEdl(const std::string& mediaFilePath, float fps);
  bool ReadComskip(const std::string& mediaFilePath, float fps);
  bool ReadVideoReDo(const std::string& mediaFilePath);
  bool ReadBeyondTV(const std::string& mediaFilePath);
  bool ReadPvr(const CFileItem& fileItem);
```

Code after:
  */
  std::optional<int> GetNextSceneMarker(Direction direction, int clock);

  static std::string MillisecondsToTimeString(int milliSeconds);

private:
  // total cut time (edl cuts) in ms
  int m_totalCutTime;
  std::vector<EDL::Edit> m_vecEdits;
  std::vector<int> m_vecSceneMarkers;

  /*!
   * @brief Last processed EDL edit time (ms)
  */
  int m_lastEditTime;

  /*!
   * @brief Last processed EDL edit action type
  */
  EDL::Action m_lastEditActionType{EDL::EDL_ACTION_NONE};

  bool ReadEdl(const std::string& mediaFilePath, float fps);
  bool ReadComskip(const std::string& mediaFilePath, float fps);
  bool ReadVideoReDo(const std::string& mediaFilePath);
  bool ReadBeyondTV(const std::string& mediaFilePath);
  bool ReadPvr(const CFileItem& fileItem);

  /*!
   * @brief Adds an edit to the list of EDL edits
   * @param newEdit the edit to add
   * @return true if the operation succeeds, false otherwise
  */
  bool AddEdit(const EDL::Edit& newEdit);

  bool AddSceneMarker(int sceneMarker);

  void MergeShortCommBreaks();

  /*!
   * @brief Adds scene markers at the start and end of some edits
   * (currently only for commercial breaks)
  */
  void AddSceneMarkersAtStartAndEndOfEdits();
};
Repository: xbmc/xbmc
File: xbmc/cores/EdlEdit.h (C/C++)

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

#pragma once

#include <chrono>

namespace EDL
{

constexpr int EDL_ACTION_NONE = -1;

enum class Action
{
  CUT = 0,
  MUTE = 1,
  SCENE = 2,
  COMM_BREAK = 3
};

struct Edit
{
  std::chrono::milliseconds start{std::chrono::milliseconds{0}};
  std::chrono::milliseconds end{std::chrono::milliseconds{0}};
  Action action = Action::CUT;
};

} // namespace EDL


Review comment (suggestion):
Could this be expressed a bit easier? Just asking, don’t know. 

 std::chrono::milliseconds start{0};

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

#pragma once

#include <chrono>

namespace EDL
{

constexpr int EDL_ACTION_NONE = -1;

enum class Action
{
  CUT = 0,
  MUTE = 1,
  SCENE = 2,
  COMM_BREAK = 3
};

struct Edit
{
  std::chrono::milliseconds start{0};
  std::chrono::milliseconds end{0};
  Action action = Action::CUT;
};

} // namespace EDL
Repository: xbmc/xbmc
File: xbmc/cores/VideoPlayer/VideoPlayer.cpp (C++)

Code before:

  CDVDMsgPlayerSeek::CMode mode;
  mode.time = (int)seekTarget;
  mode.backward = !bPlus;
  mode.accurate = false;
  mode.restore = restore;
  mode.trickplay = false;
  mode.sync = true;

  m_messenger.Put(std::make_shared<CDVDMsgPlayerSeek>(mode));
  SynchronizeDemuxer();
  if (seekTarget < 0)
    seekTarget = 0;
  m_callback.OnPlayBackSeek(seekTarget, seekTarget - time);
}

bool CVideoPlayer::SeekScene(Direction seekDirection)
{
  if (!m_Edl.HasSceneMarker())
    return false;

  /*
   * There is a 5 second grace period applied when seeking for scenes backwards. If there is no
   * grace period applied it is impossible to go backwards past a scene marker.
   */
  std::chrono::milliseconds clock = std::chrono::milliseconds(GetTime());
  if (seekDirection == Direction::BACKWARD && clock > 5s) // 5 seconds
    clock -= 5s;

  const std::optional<std::chrono::milliseconds> sceneMarker =
      m_Edl.GetNextSceneMarker(seekDirection, clock);
  if (sceneMarker)
  {
    /*
     * Seeking is flushed and inaccurate, just like Seek()
     */
    CDVDMsgPlayerSeek::CMode mode;
    mode.time = sceneMarker.value().count();
    mode.backward = seekDirection == Direction::BACKWARD;
    mode.accurate = false;
    mode.restore = false;
    mode.trickplay = false;
    mode.sync = true;

    m_messenger.Put(std::make_shared<CDVDMsgPlayerSeek>(mode));
    SynchronizeDemuxer();
    return true;
  }
  return false;
}


Review comment (suggestion):
```suggestion
  auto clock = std::chrono::milliseconds(GetTime());
```


Code after:

  CDVDMsgPlayerSeek::CMode mode;
  mode.time = (int)seekTarget;
  mode.backward = !bPlus;
  mode.accurate = false;
  mode.restore = restore;
  mode.trickplay = false;
  mode.sync = true;

  m_messenger.Put(std::make_shared<CDVDMsgPlayerSeek>(mode));
  SynchronizeDemuxer();
  if (seekTarget < 0)
    seekTarget = 0;
  m_callback.OnPlayBackSeek(seekTarget, seekTarget - time);
}

bool CVideoPlayer::SeekScene(Direction seekDirection)
{
  if (!m_Edl.HasSceneMarker())
    return false;

  /*
   * There is a 5 second grace period applied when seeking for scenes backwards. If there is no
   * grace period applied it is impossible to go backwards past a scene marker.
   */
  auto clock = std::chrono::milliseconds(GetTime());
  if (seekDirection == Direction::BACKWARD && clock > 5s) // 5 seconds
    clock -= 5s;

  const std::optional<std::chrono::milliseconds> sceneMarker =
      m_Edl.GetNextSceneMarker(seekDirection, clock);
  if (sceneMarker)
  {
    /*
     * Seeking is flushed and inaccurate, just like Seek()
     */
    CDVDMsgPlayerSeek::CMode mode;
    mode.time = sceneMarker.value().count();
    mode.backward = seekDirection == Direction::BACKWARD;
    mode.accurate = false;
    mode.restore = false;
    mode.trickplay = false;
    mode.sync = true;

    m_messenger.Put(std::make_shared<CDVDMsgPlayerSeek>(mode));
    SynchronizeDemuxer();
    return true;
  }
  return false;
}
Repository: xbmc/xbmc
File: xbmc/platform/android/MemUtils.cpp (C++)

Code before:
#include <androidjni/Context.h>

namespace KODI
{
namespace MEMORY
{

void* AlignedMalloc(size_t s, size_t alignTo)
{
  void* p;
  posix_memalign(&p, alignTo, s);

  return p;
}

void AlignedFree(void* p)
{
  free(p);
}

void GetMemoryStatus(MemoryStatus* buffer)
{
  if (!buffer)
    return;

  std::unique_ptr<CJNIActivityManager> activityManager = std::make_unique<CJNIActivityManager>(
      CJNIContext::getSystemService(CJNIContext::ACTIVITY_SERVICE));

  if (activityManager)
  {
    CJNIActivityManager::MemoryInfo info;
    activityManager->getMemoryInfo(info);
    if (xbmc_jnienv()->ExceptionCheck())
    {
      xbmc_jnienv()->ExceptionClear();
      return;
    }

    *buffer = {};
    buffer->totalPhys = static_cast<unsigned long>(info.totalMem());
    buffer->availPhys = static_cast<unsigned long>(info.availMem());
  }
}

} // namespace MEMORY
} // namespace KODI


Review comment (suggestion):
```suggestion
  auto activityManager = std::make_unique<CJNIActivityManager>(
```


Code after:
#include <androidjni/Context.h>

namespace KODI
{
namespace MEMORY
{

void* AlignedMalloc(size_t s, size_t alignTo)
{
  void* p;
  posix_memalign(&p, alignTo, s);

  return p;
}

void AlignedFree(void* p)
{
  free(p);
}

void GetMemoryStatus(MemoryStatus* buffer)
{
  if (!buffer)
    return;

  auto activityManager = std::make_unique<CJNIActivityManager>(
      CJNIContext::getSystemService(CJNIContext::ACTIVITY_SERVICE));

  if (activityManager)
  {
    CJNIActivityManager::MemoryInfo info;
    activityManager->getMemoryInfo(info);
    if (xbmc_jnienv()->ExceptionCheck())
    {
      xbmc_jnienv()->ExceptionClear();
      return;
    }

    *buffer = {};
    buffer->totalPhys = static_cast<unsigned long>(info.totalMem());
    buffer->availPhys = static_cast<unsigned long>(info.availMem());
  }
}

} // namespace MEMORY
} // namespace KODI
Repository: xbmc/xbmc
File: xbmc/pvr/addons/PVRClient.cpp (C++)

Code before:
    strParentalRatingCode = m_parentalRatingCode.c_str();
    strParentalRatingIcon = m_parentalRatingIcon.c_str();
    strParentalRatingSource = m_parentalRatingSource.c_str();
  }

  virtual ~CAddonEpgTag() = default;

private:
  static std::string GetFirstAired(const CPVREpgInfoTag& tag)
  {
    const CDateTime firstAired{tag.FirstAired()};
    if (firstAired.IsValid())
      return firstAired.GetAsW3CDate();
    return {};
  }

  const std::string m_title;
  const std::string m_plotOutline;
  const std::string m_plot;
  const std::string m_originalTitle;
  const std::string m_cast;
  const std::string m_director;
  const std::string m_writer;
  const std::string m_IMDBNumber;
  const std::string m_episodeName;
  int m_episodePartNumber;
  const std::string m_iconPath;
  const std::string m_seriesLink;
  const std::string m_genreDescription;
  const std::string m_firstAired;
  const std::string m_parentalRatingCode;
  const std::string m_parentalRatingIcon;
  const std::string m_parentalRatingSource;
};

EDL::Edit ConvertAddonEdl(const PVR_EDL_ENTRY& entry)
{
  EDL::Edit edit;
  edit.start = std::chrono::milliseconds(entry.start);
  edit.end = std::chrono::milliseconds(entry.end);

  switch (entry.type)
  {
    case PVR_EDL_TYPE_CUT:
      edit.action = EDL::Action::CUT;
      break;
    case PVR_EDL_TYPE_MUTE:
      edit.action = EDL::Action::MUTE;
      break;
    case PVR_EDL_TYPE_SCENE:
      edit.action = EDL::Action::SCENE;

Review comment (refactor):
Please remove this line. A member is only needed for `const char*`API struct members.

Code after:
    strParentalRatingIcon = m_parentalRatingIcon.c_str();
    strParentalRatingSource = m_parentalRatingSource.c_str();
  }

  virtual ~CAddonEpgTag() = default;

private:
  static std::string GetFirstAired(const CPVREpgInfoTag& tag)
  {
    const CDateTime firstAired{tag.FirstAired()};
    if (firstAired.IsValid())
      return firstAired.GetAsW3CDate();
    return {};
  }

  const std::string m_title;
  const std::string m_plotOutline;
  const std::string m_plot;
  const std::string m_originalTitle;
  const std::string m_cast;
  const std::string m_director;
  const std::string m_writer;
  const std::string m_IMDBNumber;
  const std::string m_episodeName;
  const std::string m_iconPath;
  const std::string m_seriesLink;
  const std::string m_genreDescription;
  const std::string m_firstAired;
  const std::string m_parentalRatingCode;
  const std::string m_parentalRatingIcon;
  const std::string m_parentalRatingSource;
};

EDL::Edit ConvertAddonEdl(const PVR_EDL_ENTRY& entry)
{
  EDL::Edit edit;
  edit.start = std::chrono::milliseconds(entry.start);
  edit.end = std::chrono::milliseconds(entry.end);

  switch (entry.type)
  {
    case PVR_EDL_TYPE_CUT:
      edit.action = EDL::Action::CUT;
      break;
    case PVR_EDL_TYPE_MUTE:
      edit.action = EDL::Action::MUTE;
      break;
    case PVR_EDL_TYPE_SCENE:
      edit.action = EDL::Action::SCENE;
      break;
    case PVR_EDL_TYPE_COMBREAK:
Repository: xbmc/xbmc
File: xbmc/pvr/recordings/PVRRecording.cpp (C++)

Code before:
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_parentalRating;
}

const std::string& CPVRRecording::GetParentalRatingCode() const
{
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_parentalRatingCode;
}

const std::string& CPVRRecording::GetParentalRatingIcon() const
{
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_parentalRatingIcon;
}

const std::string& CPVRRecording::GetParentalRatingSource() const
{
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_parentalRatingSource;
}

int CPVRRecording::EpisodePart() const
{
  return m_episodePartNumber;
}


Review comment (suggestion):
Add a mutex please.
```c++
std::unique_lock<CCriticalSection> lock(m_critSection);
return m_episodePartNumber;
```

Code after:
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_parentalRating;
}

const std::string& CPVRRecording::GetParentalRatingCode() const
{
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_parentalRatingCode;
}

const std::string& CPVRRecording::GetParentalRatingIcon() const
{
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_parentalRatingIcon;
}

const std::string& CPVRRecording::GetParentalRatingSource() const
{
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_parentalRatingSource;
}

int CPVRRecording::EpisodePart() const
{
  std::unique_lock<CCriticalSection> lock(m_critSection);
  return m_episodePartNumber;
}
Repository: xbmc/xbmc
File: xbmc/cores/AudioEngine/Sinks/AESinkXAudio.cpp (C++)

Code before:

  HRESULT hr = m_sourceVoice->SubmitSourceBuffer(&xbuffer);

  if (hr != S_OK)
  {
    CLog::LogF(LOGERROR, "SubmitSourceBuffer failed due to {:X}", hr);
    delete xbuffer.pContext;
    return false;
  }

  m_sinkFrames += frames;
  m_framesInBuffers += frames;
  return true;
}

XAUDIO2_BUFFER CAESinkXAudio::BuildXAudio2Buffer(uint8_t** data,
                                                 unsigned int frames,
                                                 unsigned int offset)
{
  const unsigned int dataLength{frames * m_format.m_frameSize};

  struct buffer_ctx* ctx = new buffer_ctx;
  ctx->data = new uint8_t[dataLength];
  ctx->frames = frames;
  ctx->sink = this;

  if (data)
    memcpy(ctx->data, data[0] + offset * m_format.m_frameSize, dataLength);
  else
    memset(ctx->data, 0, dataLength);

  XAUDIO2_BUFFER xbuffer{};
  xbuffer.AudioBytes = dataLength;
  xbuffer.pAudioData = ctx->data;
  xbuffer.pContext = ctx;

  return xbuffer;
}


Review comment (suggestion):
As this additional frame is a frame AE is unaware of, we need to make sure it is nowhere used in reporting back to AE as this would make AE's error computation problematic. If that's not the case and only used internally to track Flush / blocking drain operation all good.

Code after:
  ret = (str == "USB");
  PropVariantClear(&varName);
  if (pProperty)
    pProperty->Release();
#endif
  return false;
}

bool CAESinkXAudio::AddEndOfStreamPacket()
{
  constexpr unsigned int frames{1};

  XAUDIO2_BUFFER xbuffer = BuildXAudio2Buffer(nullptr, frames, 0);
  xbuffer.Flags = XAUDIO2_END_OF_STREAM;

  HRESULT hr = m_sourceVoice->SubmitSourceBuffer(&xbuffer);

  if (hr != S_OK)
  {
    CLog::LogF(LOGERROR, "SubmitSourceBuffer failed due to {:X}", hr);
    delete xbuffer.pContext;
    return false;
  }

  m_sinkFrames += frames;
  m_framesInBuffers += frames;
  return true;
}

XAUDIO2_BUFFER CAESinkXAudio::BuildXAudio2Buffer(uint8_t** data,
                                                 unsigned int frames,
                                                 unsigned int offset)
{
  const unsigned int dataLength{frames * m_format.m_frameSize};

  struct buffer_ctx* ctx = new buffer_ctx;
  ctx->data = new uint8_t[dataLength];
  ctx->frames = frames;
  ctx->sink = this;

  if (data)
    memcpy(ctx->data, data[0] + offset * m_format.m_frameSize, dataLength);
  else
    CAEUtil::GenerateSilence(m_format.m_dataFormat, m_format.m_frameSize, ctx->data, frames);

  XAUDIO2_BUFFER xbuffer{};
  xbuffer.AudioBytes = dataLength;
  xbuffer.pAudioData = ctx->data;
  xbuffer.pContext = ctx;

  return xbuffer;
Repository: xbmc/xbmc
File: xbmc/filesystem/File.cpp (C++)

Code before:

      if (aByte == traits::to_int_type('\r'))
      {
        if (m_pBuffer->sgetc() == traits::to_int_type('\n'))
          m_pBuffer->sbumpc();
        break;
      }

      line.emplace_back(traits::to_char_type(aByte));
    }

    return true;
  }

  try
  {
    // Read by buffer chuncks until to EOL or EOF
    do
    {
      char bufferLine[1025];
      if (!m_pFile->ReadString(bufferLine, 1024)) // EOF or error
        return !line.empty();

      const size_t length = std::strlen(bufferLine);
      line.reserve(line.size() + length);
      std::copy(bufferLine, bufferLine + length, std::back_inserter(line));
    } while (line.back() != '\n' && line.back() != '\r');

    return true;
  }
  XBMCCOMMONS_HANDLE_UNCHECKED
  catch (...)
  {
    CLog::Log(LOGERROR, "{} - Unhandled exception", __FUNCTION__);
  }
  return false;
}

//*********************************************************************************************
bool CFile::ReadString(char *szLine, int iLineLength)
{
  if (!m_pFile || !szLine)
    return false;

  if (m_pBuffer)
  {
    typedef CFileStreamBuffer::traits_type traits;
    CFileStreamBuffer::int_type aByte = m_pBuffer->sgetc();

    if(aByte == traits::eof())
      return false;

Review comment (suggestion):
```suggestion
      line.insert(line.end(), bufferLine, bufferLine + length);
```

Code after:

      if (aByte == traits::to_int_type('\r'))
      {
        if (m_pBuffer->sgetc() == traits::to_int_type('\n'))
          m_pBuffer->sbumpc();
        break;
      }

      line.emplace_back(traits::to_char_type(aByte));
    }

    return true;
  }

  try
  {
    // Read by buffer chuncks until to EOL or EOF
    do
    {
      char bufferLine[1025];
      if (!m_pFile->ReadString(bufferLine, sizeof(bufferLine))) // EOF or error
        return !line.empty();

      const size_t length = std::strlen(bufferLine);
      line.insert(line.end(), bufferLine, bufferLine + length);
    } while (line.back() != '\n' && line.back() != '\r');

    return true;
  }
  XBMCCOMMONS_HANDLE_UNCHECKED
  catch (...)
  {
    CLog::Log(LOGERROR, "{} - Unhandled exception", __FUNCTION__);
  }
  return false;
}

//*********************************************************************************************
bool CFile::ReadString(char *szLine, int iLineLength)
{
  if (!m_pFile || !szLine)
    return false;

  if (m_pBuffer)
  {
    typedef CFileStreamBuffer::traits_type traits;
    CFileStreamBuffer::int_type aByte = m_pBuffer->sgetc();

    if(aByte == traits::eof())
      return false;
Repository: xbmc/xbmc
File: xbmc/filesystem/File.cpp (C++)

Code before:
      {
        if (m_pBuffer->sgetc() == traits::to_int_type('\r'))
          m_pBuffer->sbumpc();
        break;
      }

      if (aByte == traits::to_int_type('\r'))
      {
        if (m_pBuffer->sgetc() == traits::to_int_type('\n'))
          m_pBuffer->sbumpc();
        break;
      }

      line.emplace_back(traits::to_char_type(aByte));
    }

    return true;
  }

  try
  {
    // Read by buffer chuncks until to EOL or EOF
    do
    {
      char bufferLine[1025];
      if (!m_pFile->ReadString(bufferLine, 1024)) // EOF or error
        return !line.empty();

      const size_t length = std::strlen(bufferLine);
      line.reserve(line.size() + length);
      std::copy(bufferLine, bufferLine + length, std::back_inserter(line));
    } while (line.back() != '\n' && line.back() != '\r');

    return true;
  }
  XBMCCOMMONS_HANDLE_UNCHECKED
  catch (...)
  {
    CLog::Log(LOGERROR, "{} - Unhandled exception", __FUNCTION__);
  }
  return false;
}

//*********************************************************************************************
bool CFile::ReadString(char *szLine, int iLineLength)
{
  if (!m_pFile || !szLine)
    return false;

  if (m_pBuffer)
  {

Review comment (suggestion):
```suggestion
      char bufferLine[1025];
      if (!m_pFile->ReadString(bufferLine, sizeof(bufferLine))) // EOF or error
```
If you only want to read 1023 bytes from the file make `bufferLine` 1024. Either way the extra unused byte is confusing.

Code after:
      {
        if (m_pBuffer->sgetc() == traits::to_int_type('\r'))
          m_pBuffer->sbumpc();
        break;
      }

      if (aByte == traits::to_int_type('\r'))
      {
        if (m_pBuffer->sgetc() == traits::to_int_type('\n'))
          m_pBuffer->sbumpc();
        break;
      }

      line.emplace_back(traits::to_char_type(aByte));
    }

    return true;
  }

  try
  {
    // Read by buffer chuncks until to EOL or EOF
    do
    {
      char bufferLine[1025];
      if (!m_pFile->ReadString(bufferLine, sizeof(bufferLine))) // EOF or error
        return !line.empty();

      const size_t length = std::strlen(bufferLine);
      line.insert(line.end(), bufferLine, bufferLine + length);
    } while (line.back() != '\n' && line.back() != '\r');

    return true;
  }
  XBMCCOMMONS_HANDLE_UNCHECKED
  catch (...)
  {
    CLog::Log(LOGERROR, "{} - Unhandled exception", __FUNCTION__);
  }
  return false;
}

//*********************************************************************************************
bool CFile::ReadString(char *szLine, int iLineLength)
{
  if (!m_pFile || !szLine)
    return false;

  if (m_pBuffer)
  {
    typedef CFileStreamBuffer::traits_type traits;