New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 9 users
Status: Archived
Owner:
Closed: Nov 7
Cc:
Components:
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment
Unit tests failing on Android
Reported by henrike@webrtc.org, Jul 3 2013 Back to list
Split this issue into several smaller issues and assign accordingly.

The following unit test fails when running on Android.

[ RUN      ] VideoProcessingModuleTest.Denoising
Never finishes.

[ RUN      ] DecoderDatabase.GetDecoder
../../third_party/webrtc/modules/audio_coding/neteq4/decoder_database_unittest.cc:73: Failure
Value of: db.RegisterPayload(kPayloadType, kDecoderILBC)
  Actual: -2
Expected: DecoderDatabase::kOK
Which is: 0
../../third_party/webrtc/modules/audio_coding/neteq4/decoder_database_unittest.cc:75: Failure
Value of: dec != NULL
  Actual: false
Expected: true
[  FAILED  ] DecoderDatabase.GetDecoder (1 ms)

[ RUN      ] NetEqExternalDecoderTest.RunTest
 9:35:47:138 (neteq_impl.cc:97): Create NetEqImpl object with fs = 32000.
 9:35:47:138 (neteq_impl.cc:97): Create NetEqImpl object with fs = 32000.
 9:35:47:140 (neteq_impl.cc:200): InsertExternal failed: rtp_payload_type=_, codec=10
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_external_decoder_unittest.cc:81: Failure
Value of: neteq_external_->RegisterExternalDecoder(external_decoder_, decoder, sample_rate_hz_, kPayloadType)
  Actual: -1
Expected: NetEq::kOK
Which is: 0
 9:35:47:140 (neteq_impl.cc:113): Deleting NetEqImpl object.
 9:35:47:140 (neteq_impl.cc:113): Deleting NetEqImpl object.
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_external_decoder_unittest.cc:74: Failure
Actual function call count doesn't match EXPECT_CALL(*external_decoder_, Init())...
         Expected: to be called once
           Actual: never called - unsatisfied and active
[  FAILED  ] NetEqExternalDecoderTest.RunTest (3 ms)

[ RUN      ] NetEqDecodingTest.TestBitExactness
 9:35:47:148 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:148 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0
 9:35:47:149 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.TestBitExactness (1 ms)

[ RUN      ] NetEqDecodingTest.TestNetworkStatistics
 9:35:47:149 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:149 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0
 9:35:47:150 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.TestNetworkStatistics (1 ms)

[ RUN      ] NetEqDecodingTest.TestFrameWaitingTimeStatistics
 9:35:47:150 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:150 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0
 9:35:47:151 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.TestFrameWaitingTimeStatistics (1 ms)

[ RUN      ] NetEqDecodingTest.TestAverageInterArrivalTimeNegative
 9:35:47:151 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:151 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0
 9:35:47:151 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.TestAverageInterArrivalTimeNegative (1 ms)

[ RUN      ] NetEqDecodingTest.TestAverageInterArrivalTimePositive
 9:35:47:152 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:152 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0
 9:35:47:152 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.TestAverageInterArrivalTimePositive (1 ms)

[ RUN      ] NetEqDecodingTest.LongCngWithClockDrift
 9:35:47:153 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:153 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0
 9:35:47:153 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.LongCngWithClockDrift (1 ms)

[ RUN      ] NetEqDecodingTest.UnknownPayloadType
 9:35:47:154 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:154 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0

 9:35:47:154 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.UnknownPayloadType (0 ms)

[ RUN      ] NetEqDecodingTest.DecoderError
 9:35:47:155 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:155 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0
 9:35:47:155 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.DecoderError (0 ms)

[ RUN      ] NetEqDecodingTest.GetAudioBeforeInsertPacket
 9:35:47:156 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:35:47:156 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=f, codec=4
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc:233: Failure
Value of: neteq_->RegisterPayloadType(kDecoderILBC, 102)
  Actual: -1
Expected: 0

 9:35:47:156 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] NetEqDecodingTest.GetAudioBeforeInsertPacket (1 ms)

[ RUN      ] ApmTest.Process
Running test 1 of 12...
Running test 2 of 12...
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1514: Failure
Value of: has_voice_count
  Actual: 613
Expected: test->has_voice_count()
Which is: 607
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1518: Failure
Value of: max_output_average
  Actual: 2792
Expected: test->max_output_average()
Which is: 2781
Running test 3 of 12...
Running test 4 of 12...
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1514: Failure
Value of: has_voice_count
  Actual: 613
Expected: test->has_voice_count()
Which is: 607
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1518: Failure
Value of: max_output_average
  Actual: 2792
Expected: test->max_output_average()
Which is: 2781
Running test 5 of 12...
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1514: Failure
Value of: has_voice_count
  Actual: 572
Expected: test->has_voice_count()
Which is: 571
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1518: Failure
Value of: max_output_average
  Actual: 3136
Expected: test->max_output_average()
Which is: 3119
Running test 6 of 12...
Running test 7 of 12...
Running test 8 of 12...
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1518: Failure
Value of: max_output_average
  Actual: 2763
Expected: test->max_output_average()
Which is: 2752
Running test 9 of 12...
Running test 10 of 12...
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1518: Failure
Value of: max_output_average
  Actual: 2763
Expected: test->max_output_average()
Which is: 2752
Running test 11 of 12...
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1514: Failure
Value of: has_voice_count
  Actual: 592
Expected: test->has_voice_count()
Which is: 590
../../third_party/webrtc/modules/audio_processing/test/unit_test.cc:1518: Failure
Value of: max_output_average
  Actual: 3085
Expected: test->max_output_average()
Which is: 3067
Running test 12 of 12...
[  FAILED  ] ApmTest.Process (7130 ms)

[ RUN      ] MultiChannel/NetEqStereoTestNoJitter.RunTest/0(-23)
 9:36:37:495 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:495 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:496 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=_, codec=8
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_stereo_unittest.cc:127: Failure
Value of: neteq_mono_->RegisterPayloadType(mono_decoder, kPayloadTypeMono)
  Actual: -1
Expected: NetEq::kOK
Which is: 0
 9:36:37:496 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] MultiChannel/NetEqStereoTestNoJitter.RunTest/0, where GetParam() = {frame_size = 10, num_channels = 2, sample_rate = 8000} (2 ms)
 9:36:37:496 (neteq_impl.cc:113): Deleting NetEqImpl object.

[ RUN      ] MultiChannel/NetEqStereoTestPositiveDrift.RunTest/0(-23)
 9:36:37:544 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:544 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:545 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=_, codec=8
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_stereo_unittest.cc:127: Failure
Value of: neteq_mono_->RegisterPayloadType(mono_decoder, kPayloadTypeMono)
  Actual: -1
Expected: NetEq::kOK
Which is: 0
 9:36:37:545 (neteq_impl.cc:113): Deleting NetEqImpl object.
 9:36:37:545 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] MultiChannel/NetEqStereoTestPositiveDrift.RunTest/0, where GetParam() = {frame_size = 10, num_channels = 2, sample_rate = 8000} (2 ms)

[ RUN      ] MultiChannel/NetEqStereoTestNegativeDrift.RunTest/0 (-23)
 9:36:37:594 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:594 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:595 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=_, codec=8
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_stereo_unittest.cc:127: Failure
Value of: neteq_mono_->RegisterPayloadType(mono_decoder, kPayloadTypeMono)
  Actual: -1
Expected: NetEq::kOK
Which is: 0
 9:36:37:595 (neteq_impl.cc:113): Deleting NetEqImpl object.
 9:36:37:596 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] MultiChannel/NetEqStereoTestNegativeDrift.RunTest/0, where GetParam
m() = {frame_size = 60, num_channels = 2, sample_rate = 32000} (2 ms)

[ RUN      ] MultiChannel/NetEqStereoTestDelays.RunTest/0 (-23)
 9:36:37:644 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:645 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:646 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=_, codec=8
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_stereo_unittest.cc:127: Failure
Value of: neteq_mono_->RegisterPayloadType(mono_decoder, kPayloadTypeMono)
  Actual: -1
Expected: NetEq::kOK
Which is: 0
 9:36:37:646 (neteq_impl.cc:113): Deleting NetEqImpl object.
 9:36:37:646 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] MultiChannel/NetEqStereoTestDelays.RunTest/0, where GetParam() = {frame_size = 10, num_channels = 2, sample_rate = 8000} (2 ms)

[ RUN      ] MultiChannel/NetEqStereoTestLosses.RunTest/0(-23)
 9:36:37:695 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:695 (neteq_impl.cc:97): Create NetEqImpl object with fs = 8000.
 9:36:37:696 (neteq_impl.cc:167): RegisterPayload failed: rtp_payload_type=_, codec=8
../../third_party/webrtc/modules/audio_coding/neteq4/neteq_stereo_unittest.cc:127: Failure
Value of: neteq_mono_->RegisterPayloadType(mono_decoder, kPayloadTypeMono)
  Actual: -1
Expected: NetEq::kOK
Which is: 0
 9:36:37:697 (neteq_impl.cc:113): Deleting NetEqImpl object.
 9:36:37:697 (neteq_impl.cc:113): Deleting NetEqImpl object.
[  FAILED  ] MultiChannel/NetEqStereoTestLosses.RunTest/0, where GetParam() = {frame_size = 10, num_channels = 2, sample_rate = 8000} (2 ms)

 
Comment 1 by henrike@webrtc.org, Jul 8 2013
Seems I missed one test or it broke recently:
[ RUN      ] MediaFileTest.StartPlayingAudioFileWithoutError
11:52: 7: 27 Could not open input file /storage/emulated/0/data/voice_engine/audio_tiny48.wav
../../third_party/webrtc/modules/media_file/source/media_file_unittest.cc:40: Failure
Value of: media_file_->StartPlayingAudioFile( audio_file.c_str(), 0, false, webrtc::kFileFormatWavFile)
  Actual: -1
Expected: 0
[  FAILED  ] MediaFileTest.StartPlayingAudioFileWithoutError (1 ms)

Comment 2 by henrike@webrtc.org, Jul 25 2013
New batch (enabling more tests):
common_video_unittests:
  TestScaler.PointScaleTest
  TestScaler.BiLinearScaleTest
  TestScaler.BoxScaleTest

metrics_unittests:
  VideoMetricsTest.ReturnsPerfectResultForIdenticalFilesPSNR
  VideoMetricsTest.ReturnsPerfectResultForIdenticalFilesSSIM
  VideoMetricsTest.ReturnsPerfectResultForIdenticalFilesBothMetrics
  VideoMetricsTest.MissingReferenceFilePSNR
  VideoMetricsTest.MissingReferenceFileSSIM
  VideoMetricsTest.MissingReferenceFileBothMetrics
  VideoMetricsTest.MissingTestFilePSNR
  VideoMetricsTest.MissingTestFileSSIM
  VideoMetricsTest.MissingTestFileBothMetrics
  VideoMetricsTest.EmptyFilesPSNR
  VideoMetricsTest.EmptyFilesSSIM
  VideoMetricsTest.EmptyFilesBothMetrics

modules_integrationtests:
  DualStreamTest.BitExactSyncMonoInputMonoPrimaryWb20Ms
  DualStreamTest.BitExactSyncMonoInput_StereoPrimaryWb20Ms
  DualStreamTest.BitExactSyncMonoInputMonoPrimarySwb20Ms
  DualStreamTest.BitExactSyncMonoInputStereoPrimarySwb20Ms
  DualStreamTest.BitExactSyncMonoInputMonoPrimaryWb40Ms
  DualStreamTest.BitExactSyncMonoInputStereoPrimaryWb40Ms
  DualStreamTest.BitExactSyncStereoInputMonoPrimaryWb20Ms
  DualStreamTest.BitExactSyncStereoInputStereoPrimaryWb20Ms
  DualStreamTest.BitExactSyncStereoInputMonoPrimarySwb20Ms
  DualStreamTest.BitExactSyncStereoInputStereoPrimarySwb20Ms
  DualStreamTest.BitExactSyncStereoInputMonoPrimaryWb40Ms
  DualStreamTest.BitExactSyncStereoInputStereoPrimaryWb40Ms
  DualStreamTest.BitExactAsyncMonoInputMonoPrimaryWb20Ms
  DualStreamTest.BitExactAsyncMonoInputMonoPrimaryWb40Ms
  DualStreamTest.Api
  TargetDelayTest.OutOfRangeInput
  TargetDelayTest.NoTargetDelayBufferSizeChanges
  TargetDelayTest.WithTargetDelayBufferNotChanging
  TargetDelayTest.RequiredDelayAtCorrectRange
  AudioCodingModuleTest.RunAllTests
  InitialPlayoutDelayTest.NbMono
  InitialPlayoutDelayTest.WbMono
  InitialPlayoutDelayTest.SwbMono
  InitialPlayoutDelayTest.NbStereo
  InitialPlayoutDelayTest.WbStereo
  InitialPlayoutDelayTest.SwbStereo
  VideoProcessorIntegrationTest.ProcessZeroPacketLoss
  VideoProcessorIntegrationTest.Process5PercentPacketLoss
  VideoProcessorIntegrationTest.Process10PercentPacketLoss
  VideoProcessorIntegrationTest.ProcessNoLossChangeBitRate
  VideoProcessorIntegrationTest.ProcessNoLossChangeFrameRateFrameDrop
  VideoProcessorIntegrationTest.ProcessNoLossSpatialResizeFrameDrop
  VideoProcessorIntegrationTest.ProcessNoLossTemporalLayers
  TestVp8Impl.BaseUnitTest
  TestVp8Impl.AlignedStrideEncodeDecode

neteq_unittests:
  NetEqDecodingTest.TestBitExactness

test_support_unittests:
  FileUtilsTest.OutputPathFromRootWorkingDir

voice_engine_unittests:
  VoECodecTest.DualStreamSetSecondaryBeforePrimaryFails
  VoECodecTest.DualStreamRegisterWithWrongInputsFails
  VoECodecTest.DualStreamGetSecodaryEncoder
  VoECodecTest.DualStreamRemoveSecondaryCodec
Project Member Comment 3 by andrew@webrtc.org, Jul 26 2013
This list is beginning to look pretty frightening. I think we'll need to get fixing these on OKRs for next quarter.
Project Member Comment 4 by kjellander@webrtc.org, Sep 5 2013
Can we split up this bug into smaller bugs that are more focused? That way I think it's more likely engineers will take a look and possibly fix.
I fixed the metrics_unittests test by just changing the path the test tried to write a file to from the current working dir to the webrtc::test::OutputPath() (see r4634). That resolved all the crashes in that test.

I assume there may be more that are as easily fixed as that one, if someone just take a look. 
Project Member Comment 5 by kjellander@webrtc.org, Sep 5 2013
Cc: kjellander@webrtc.org
Project Member Comment 6 by bugdroid1@chromium.org, Sep 24 2013
The following revision refers to this bug:
    http://code.google.com/p/webrtc/source/detail?r=4829

------------------------------------------------------------------------
r4829 | andrew@webrtc.org | 2013-09-24T18:43:28.445702Z

Changed paths:
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc&spec=svn4829&r_previous=4828&r=4829&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/modules/video_coding/main/source/video_sender_unittest.cc&spec=svn4829&r_previous=4828&r=4829&format=side

Disable some VP8 tests on Android.

DecodeWithACompleteKeyFrame and FixedTemporalLayersStrategy.

TBR=andresp
BUG= 2037 

Review URL: https://webrtc-codereview.appspot.com/2283004
------------------------------------------------------------------------
Project Member Comment 7 by kjellander@webrtc.org, Dec 2 2013
Any progress on this? Are there plans to get this done in Q4 and/or should we file separate issues?
Project Member Comment 8 by andrew@webrtc.org, Dec 2 2013
It's likely very simple to fix the AudioCoding tests; Turaj might have a look at that soon. Otherwise there are no plans to address this in Q4. Please feel free to reorganize or take it over as your time permits.
Project Member Comment 9 by henrik.lundin@webrtc.org, May 14 2014
Cc: henrik.lundin@webrtc.org
Project Member Comment 10 by henrik.lundin@webrtc.org, May 14 2014
Re-enabling the NetEq tests is tracked here: https://code.google.com/p/webrtc/issues/detail?id=3343.

Project Member Comment 11 by bjornv@webrtc.org, Jun 2 2014
Cc: bjornv@webrtc.org
Comment 12 by vrk@webrtc.org, Oct 15 2014
Labels: Area-Audio Area-Video
Comment 13 by vrk@webrtc.org, Oct 15 2014
Labels: OS-Android
Project Member Comment 14 by mflodman@webrtc.org, Dec 10 2014
Labels: -Area-Video
Project Member Comment 15 by tina.legrand@webrtc.org, Dec 11 2014
Labels: EngTriaged IceBox
Project Member Comment 16 by bjornv@webrtc.org, Jun 9 2015
Cc: -bjornv@webrtc.org
Project Member Comment 17 by andrew@webrtc.org, Nov 3 2015
Owner: henrik.lundin@webrtc.org
Not sure if this is still a problem. Over to Mr. Lundin for triaging.
Project Member Comment 18 by henrik.lundin@webrtc.org, Nov 25 2015
Labels: fixit
Project Member Comment 19 by ivoc@webrtc.org, Dec 7 2015
Owner: ivoc@webrtc.org
Status: Started
Project Member Comment 20 by ivoc@webrtc.org, Dec 8 2015
Owner: henrik.lundin@webrtc.org
Status: Assigned
Project Member Comment 21 by kjellander@webrtc.org, Dec 1 2016
Cc: -henrike@webrtc.org
Project Member Comment 22 by tina.legrand@webrtc.org, Nov 7
Status: Archived
[Bulk edit] This issue hasn't been modified the last twelve months -> archiving.

If this is still a valid issue that should be open, please reopen again.
Sign in to add a comment